On 12/13/12 23:01, Tim Foster wrote:
Hi Shawn,
Thanks for the thorough review, much appreciated. I've spent some time
reworking the internals, and have a new webrev and incremental at:
https://cr.opensolaris.org/action/browse/pkg/timf/pkgrepo-verify-1/
src/pkgrepo.py:
lines 1231-1236: The formatting here is a bit wonky.
line 1246: Why wouldn't this be an "else:" case at 1263 instead?
line 1269: You sure the default width of 70 is the right wrapping
width? How well did textwrap appear to work?
line 1293: drop the "else:" and de-indent following line; makes
it obvious no code after will execute
line 1314-1315: if we decide to support specifying specific packages
later, how much would that affect everything? trivially?
line 1438: I would consider making the fact that repairs happened
more obvious by changing the wording, perhaps: "Repository repairs
completed."
general: I noticed you chose to let the progress tracker do the
output, instead of having the client do the output like pkg(1) does;
that isn't wrong necessarily, but it is an interesting choice. What was
the reasoning? Should we also change pkg(1) to do the same?
src/tests/pkg5unittest.py"
line 2130: this should move to the top of the file with the rest of
the imports; I didn't mean the placement to be literal :-)
How close is the output of pkgrepo verify/fix to pkg(1)? Can you
example output for some various cases?
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
