[resending due to delivery failure on the first attempt]
William,
installadm.py
remove spaces after [, { and before ],} - line 139, 210, elsewhere
Is --debug option forwarded to subcommands?
It isn't forwarded, it changes the log level which is global.
53: this value should be commented and a symbolic value used if available
Line is unused, so will remove it.
85: use Args:/Raises: standard for commenting
92-6: please explain in comments
Will remove unused lines.
98: logging should go to stderr in principle, not stdout, since some installadm
subcommands have output that can be handled or redirected independently: e.g.,
export.
Will change.
170: can only root do this service management? Couldn't some other user who had
user profile settings of 'primary administrator', for example, do this?
This conversion maintains existing permission handling as in installadm.c.
Although changing this might be desirable, it is outside the scope of these
changes.
199 repeated usage of 'status' as a constant string suggests a symbolic value be
used instead and perhaps have a comment with it
I'll add this (and the other property keys) to ai_smf_service.
277: explain what this does in comments
ok
296: what will happen if two different subcommands on the command line?
Shouldn't the subcommand always be in argv[1]?
The first subcommand will reject the second subcommand as an argument.
There can be options as well.
Updated webrev at:
full: http://cr.opensolaris.org/~sohn/7015429.v2
diffs: http://cr.opensolaris.org/~sohn/7015429.v2.diffs
Thanks for the review,
Sue
I'll continue to look at other modules.
William
On 01/28/11 07:57 PM, Sue Sohn wrote:
Could I please get a review of the changes for:
7015429 Convert installadm.c to python
Webrev:
http://cr.opensolaris.org/~sohn/7015429
We can thank Joe for some of this code, since he began the conversion effort as
part of the ISIM project.
Thanks,
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
