On Wed, Oct 08, 2008 at 10:30:10AM -0500, Shawn Walker wrote:
> Dan Price wrote:
> > Folks,
> > 
> > I have the following fixes for you to review:
> > 
> > 3777 PKG_TIMEOUT_MAX is a placebo setting
> > 3778 We can be more informative and accurate when timeouts happen
> > 3779 Some KeyboardInterrupt exception handling can be cleaner
> > 
> > http://cr.opensolaris.org/~dp/pkg-timeout
> 
> General concerns:
> How are we going to deal with localisation of messages that are 
> generated within our apis instead of from client.py?  I'm not asking you 
> to address that in this patch, but a general idea would be nice.
> 
> client.py:
>    line 1959: The str() around 'e' isn't needed given the '%s' construct?
> 
> misc.py:
>    line 424: what about having global_settings in pkg.client instead? 
> it would seem a better home than "misc" for something central to the client.
> 
>    line 425: A docstring for the class would be nice here.
> 
>    line 442: s/in this case/in this case,/
> 
>    line 449: Maybe an assert here to verify that exceptions, as passed 
> in, is actually a list?  That or just remove the ability to specify this 
> as a parameter since we don't seem to use it anywhere.
> 
>    line 522: extraneous newline?
> 
> Otherwise, nicely done.

Looks great.  I don't have any additional comments -- though +1 to the comment
about moving it from misc :)

-Brad
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to