Thanks, Jesse. I really appreciate you taking the time to review this.
Comments inline.
Harold
On 03/12/12 03:56 PM, Jesse Butler wrote:
Hi Harold-
Taking a quick look at this in the airport between flights, before I
get too much into what's happening I have a quick comment about
DiskMatches. It seems that it might be better to pass it a new disk
for the list, and have the list append and the counter increment
happen inside the object, rather than as separate functions called on
the object. Do you agree?
That's a good idea. I'll do that.
Also, overall would it be possible to trim some of the comments? I
know that comments are good, and this seems kind of a backward
request, but there are several "I'm going to do this now" sort of
comments that could probably be trimmed down.
I generally write methods by starting with the comments and then writing
the code. I will look to see if there are any that are just begging to
be removed but to be honest would much rather err on the side of too
many comments.
I'll get to more tonight or tomorrow, thanks,
Jesse
On 3/9/12 6:13 PM, Harold Shaw wrote:
Happy Friday,
Can I get a code review for the following bug:
CR: http://monaco.sfbay/detail.jsf?cr=7147174
There is an additional change that isn't noted in the webrev. I
moved line 194 in target_selection.py left by one tab. The function,
__pretty_print_disk, if called with a list of disks would only create
a string with the first disk in the list. The return statement
needed to be moved out of the loop.
Webrev: https://cr.opensolaris.org/action/browse/caiman/hshaw/7147174/
Testing:
pep8 clean
target selection unit tests have been run
new unit tests added
Full installs run on the default manifest and 2 representative manifests
autoinstall -i -m run on a set of manifests developed to test the
changes
Note: Some have been added to the unit tests and some forwarded to
QE to include in a new, more easily run AI test suite.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
