Hi Mary.
General comments:
1) A "\" is not needed at the ends of lines which are split inside (),
{}, or []. Many comments below are related to that.
2) If there are many of the same "pylint disable-msg" please consider
putting one at the top of the file instead of multitudes of them
throughout the file, to make things more readable. (Take care with this
though as you will disable this everywhere in the file instead of at
specific points.)
Specific comments below:
create_iso.py:
I don't understand the comment on 165. What does it think is not
callable, and where are we checking this already?
execution_checkpoint.py:
127 and 221: "\" not needed as already in ()
usr/src/lib/install_engine/__init__.py:
1000: Need additional () grouping pause_before and start_from. This
should have been done before...
1014 and 1018: "\" not necessary as already in ()
attributes.py:
134: "\" not needed as already in ()
Also, the code is more readable to me if it is split at the first %
instead, but it's up to you.
209, 211, 213, 215, 217, 425: "\" not needed as already in ()
425: the code is more readable to me if it is split at the first %
instead, but it's up to you.
nvl.py:
91, 182: "\" is incorrect here as this is a comment literal.
There are so many disable-msg=E1101 it seems reasonable to put one at
the top of the file (probably just below the imports) to take care of
the whole file instead. So many of them makes the file harder to read.
792: "\" not needed as already in ()
792: Nit: I know you didn't create this initial message, but I would
change the message to something more understandable, like "val: %d not
within range of 4-byte int"
822: same comments as 792, except instead of saying "4-byte int" say
"long int".
1017: same comments as 792, except instead of saying "4-byte int" say
"short int"
errsvc.py:
94, 95, 99, 100, 104, 105, 108, 215: "\" not needed as already in ()
Thanks,
Jack
On 04/25/11 07:32 PM, Mary Ding wrote:
Hi:
Can I get a code review for the following bug fix:
7036478 Additional slim_code_cleaness failures after TI/TD CUD
putbackAdditional slim_code_cleaness failures
http://monaco.sfbay.sun.com/detail.jsf?cr=7036478
Web rev is here:
http://cr.opensolaris.org/~md5645/7036478/
I had run the slim install unit tests and there are no additional
failures.
Thanks !!!
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
