On Fri 26 Sep 2008 at 03:42PM, Brock Pytlik wrote:
> Shawn Walker wrote:
> > Brock Pytlik wrote:
> >> Webrev:
> >> http://cr.opensolaris.org/~bpytlik/ips-3618-v1/src/packagemanager.py.wdiff.html
> >>
> >>
> >>
> >> Bug:
> >> Manifest retrieval error needs more info and to not show a traceback
> >> http://defect.opensolaris.org/bz/show_bug.cgi?id=3618
> >>
> >> While I'd like to fix the source of the underlying bug (1133), I feel
> >> like we have little way of telling what the actual problem. Since the
> >> problem is hard to reproduce, I'm suggesting we try this approach to
> >> allow us to gather more information when problems are encountered. It
> >> also removes the traceback from what the user is shown.
> >
> > I think you meant:
> > http://cr.opensolaris.org/~bpytlik/ips-3618-v1/
> >
> > client.py:
> > lines 2189, 2190: wording suggestion: "An error was encountered
> > while attempting to retrieve package or file data for the requested
> > operation."
> >
> > retrieve.py:
> > lines 35,42: s/Retrival/Retrieval/ fixing this here means changing
> > it everywhere else, of course
> >
> > lines 36, 42: docstrings are appreciated
> >
> > Otherwise, looks fine.
> >
> > installupdate.py:
> > line 364: I know this isn't your fault, but can you:
> > s/Attemping/Attempting/
> >
> > Cheers,
> New webrev reflecting shawn's comments at:
> http://cr.opensolaris.org/~bpytlik/ips-3618-v2/
I've reviewed this. It looks good. Just cleanup one nit
(I don't need to see the review again).
In client.py, the other except stanzas are for some reason
using '__e' for some reason. Could you either fix them to
be consistent with your thing, or vice versa? I assume the
idea here is to avoid introducing variables into the global
namespace, since we're not in any function here
(although it seems to me that it would have been easier to
just create a function...).
I presume that pull.py is suffiently different that it doesn't
need to change to use this new exception?
-dp
--
Daniel Price - Solaris Kernel Engineering - [EMAIL PROTECTED] - blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
