Dermot, I think this is fine, then. Especially since 4016 is in the top_60 and will hopefully be addressed in a reasonable time frame.
- Keith Dermot McCluskey wrote: > Keith, > > Thanks for reviewing. > > I was following the precedent of how the existing code works, > which already raises SystemExit 3 times in the same function > for similar types of error. > > If we want to change this behavior, I think it's best to tackle > it all in one go, eg via 4016 as you suggest, rather than adding > new code that looks and acts differently to the existing code. > > If you have no objections to this, I'll push my change. > > > Thanks, > - Dermot > > > > On 01/15/10 23:01, Keith Mitchell wrote: >> Hi Dermot, >> >> publish-manifest.py: >> >> 1054: I don't think raising a SystemExit error from a class method is >> the best solution. That said, I realize we have a lot of other code >> here that does raise SystemExits (and a bug against it - 4016). That >> said, it'd be a step in the right direction if the ValueError were >> caught at line 97, where the DataFiles are being initialized, and >> converted to a SystemExit there. >> >> Other than that (and that's something that in theory could be >> deferred to bug 4016 if it's not worth it), it looks good. >> >> - Keith >> >> Dermot McCluskey wrote: >>> Hi, >>> >>> Can I get a code review for: >>> >>> 12055 publish-manifest: returns a stack trace if a >>> malformed MAC address or IP address is provided in a >>> manifest >>> >>> (installadm) >>> >>> >>> Webrev is here: >>> http://cr.opensolaris.org/~dermot/webrev-12055-01/ >>> >>> >>> >>> Thanks, >>> - Dermot >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
