Sue,
Please find my response inline. Took your suggestions unless otherwise noted.
current webrev: http://cr.opensolaris.org/~wmsch/profile4/
differences with last webrev http://cr.opensolaris.org/~wmsch/profile3-diff/
On 03/ 1/11 10:20 PM, Sue Sohn wrote:
... 658, 677 get_service_info should raise non-SystemExit exceptions and the
higher level should catch them and raise a SystemExit if appropriate.
Why? In all cases, a SystemExit seems in order, since no installadm application
that calls it can function without a database.
I believe it is considered good python practice to not assume that the caller always wants to exit. It should be the caller's job
to make that determination.
I still think that in this case, since exception handling code would be replicated for 5+ scripts with no benefit to the user, just
exiting at that point is preferable.
Thanks,
William
Sue
Thank you again,
William
publish_manifest.py
147 Since you added get_service_info() in AIdb, why not call it here?
>set_criteria.py
47 Append "..." to <profile_name>
common_profile.py
Added some blank lines to separate segments of code in distinctly different
phases.
Please add blank lines between functions per pep8 guidelines.
create_profile.py
General (pep8): Separate top-level function and class definitions with two
blank lines
79 (pep8) line up string under string on line above
191 Comment here and on 194 essentially the same
Not addressed (now 194 and 197)
201 line up under profile_name
(pep8, please line up, now 204)
installadm.1m.txt
Looking much better.
30 Should "-p <profile_name>" have "..." appended?
50 -n should come before -p, to be consistent with command usage
404 Should this be profile(s), similar to 409?
439 Indicates you can have multiple profile_names, but usage
487 rather than "for client installation", maybe say
"associated with profile to export", since that is more relevant here.
566-592 Nice addition
568-569 Reword so as not to use "different" twice
592 no limit to either -> no limit to one
596-597 for a manifest -> for a manifest or profile
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
