Drew,
Thanks for the review. I have commented on the installadm-convert part of the
review inline.
Harold
usr/src/cmd/installadm/installadm-convert.py
--------------------------------------------
34: no need for this import. It's covered by the import of os
Removed.
205-215: Haha. Polo! Polo! Polo! Oh ... wait .. macro != Marco ...
Can we*please* do something about the naming in this function? This is all
but impossible to read. I honestly can't even tell you what it's doing.
I have changed the names and added comments to make the operation of the
function more clear.
266-277: Oh wow. Polo!
See above.
See above.
493: How about something easier to read?
return filter(bool, self.keys())
Done.
711: use += 1
Done.
A*lot* of this file is hard to read due to some unnecessary compression
of the lines. Between the borderline excessive list comprehensions the
word 'macro' a hobillion times, the file seems really unmaintainable.
A lot of the code in here is directly stolen from the pre-ISIM
slim_source installadm base. That said, I can talk with you off line to
get more detail on what should be changed both to educate me as well as
to make this and future code better.
I'm not real sure what to suggest at this point, though. It's super
late and we need to get this back, but if it goes back in this form,
perhaps a low priority bug could be filed for later clean-up (for
legibility, not for features).
-Drew
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
