Danek Duvall wrote: > On Sun, Oct 05, 2008 at 06:28:22PM -0500, Shawn Walker wrote: > >> http://cr.opensolaris.org/~swalker/pkg-3715-2/ > > Wow, that's a bit more code. :)
:) > client.py: > > - line 1934: You've stopped testing __img; does that mean you're > confident it won't be None here? It seems like it, but it's a change, > so I'm just making sure. Yes, __img should be checked; nice spot. > history.py: > > - line 393: we haven't been strict about this, but we should think about > being more careful about not accidentally capturing KeyboardInterrupt. Oops. Definitely not intentional. Thanks. > - line 401, 405: wouldn't these be better as assert()s? These tests are > checking for other code being broken, not a normal error condition, as > far as I can tell. I was primarily adding them to make it easier for us whenever something was broken to find out why there was a problem writing the history file since minidom is so horrid. However, asserts will do just as well here for those purposes. It is indeed not a normal error condition. Updated webrev: http://cr.opensolaris.org/~swalker/pkg-3715-3/ diff from last webrev: http://cr.opensolaris.org/~swalker/pkg-3715-3/v2-v3.patch Thanks, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
