On Fri, Jul 25, 2008 at 09:52:46AM -0700, Danek Duvall wrote: > On Thu, Jul 24, 2008 at 04:09:43PM -0700, Brad Hall wrote: > > > Link to new CR: http://cr.opensolaris.org/~bhall/bug-2627-1/ > > We use getopt in the rest of the code, rather than optparse. Unless > there's something that optparse really buys you, please use getopt.
OK, fixed. > I think -p is more consistent with the rest of solaris than -m (p for > parseable). Particularly your use of "mach" as a short form throughout, it > strongly means "machine type" to me, as in the command "mach". Makes sense, fixed that. > Is it worth assuming that the baseline for a test is "pass", and keeping > the baseline files shorter? I think that may confuse things at some point if we decide to print out a message like "test X wasn't in the baseline, you probably need to regenerate", so probably not worth it at this point. > baseline.py: > > - line 49: perhaps just make the generate default to False, and pass in > True instead of 1? Or you could do bool(generate) here. Similarly on > line 74, I think. Good point, switched to use boolean. > - line 63: return self.results.get(name, None) Didn't realize you could do that, fixed. > - line 87: "if lst" > > - line 105, 124: I don't think this work. This will catch exceptions of > type IOError or type e. > > - line 106: "storing" > > - line 130: no need for parens on the lhs OK, got all those too. Thanks for reviewing, I'll post another webrev shortly. Thanks, Brad _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
