Hi Danek & Sean, Thanks for the code reviews, I've updated the webrev at the same url.
On Wed, 2010-01-27 at 17:19 -0800, Danek Duvall wrote: > Tim Foster wrote: > > > http://cr.opensolaris.org/~timf/14038,14039,14133-pkgsend/ > > publish.py: > > - line 209, et al: shouldn't need str() Yep. > - line 389: while it makes some sense to abandon a "publish" and return > an error, here in "include", I think it makes more sense to just keep > going after warning. There's no way to undo just what succeeded in the > incomplete "include", and there's no way to resume where you left off, > so you're kinda stuck. If you really want the command to fail, allow > it to be ignorable, by returning 3, which is the "partial success" code > we've been using. I agree - I've done the last suggestion, warning (and not including the "unknown" action in the transaction) and eventually returning 3. > - line 434, 455: no need for the parens around action and err. Ok. > - line 436: in keeping with "publish", shouldn't you be abandoning the > transaction and returning an error? Yes, I've done that. > t_pkgsend.py: > > - why use a dictionary with ".1", ".2" strings, rather than creating an > empty list before line 560, and appending each path to the list as you > create it? What's there seems convoluted, if correct. Yep, that looked weird to me too now that I look at it in the cold light of day, sorry :-/ cheers, tim _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
