On 03/ 1/11 11:03 AM, Keith Mitchell wrote:
On 03/ 1/11 10:11 AM, Martin Widjaja wrote:
Sue - In reviewing this quickly, it looks good overall, but I have several
questions, mostly to satisfy my own curiosity:
I know you asked Sue, but I have some answers to chime in with here.
Martin,
Thanks for the review.
Responding to the one comment that wasn't already addressed...
- Makefile: would it be worth keeping the generic rules for building .c, etc.?
Maybe this can be useful for other things later on?
The information can be obtained from mercurial, if needed.
Thanks,
Sue
- installadm.py: 354: is there a reason to remove the traceback limit here? Is
this going to be too much if everything is printed?
If we reach a point where we're printing a traceback, we're already in a
scenario where the user encountered a bug. More information is better than less,
in that case (and the limit=2 almost always cut out the useful portion of the
traceback, since it cuts out the 'bottom' of the stack).
- installadm_common.py: 95-105: is this ksh execution to determine multi-home
going to persist or is there a better way to gather this information using
either python lib or C lib? I am personally not a fan of command line output
parsing.
I was thinking of swapping that out with ISIM, though it may be quick to change
as part of this push. The concern I have with doing it *right* now is it would
be easy to miss edge cases - by porting this right across from the .c
implementation, we're at least not introducing any new bugs.
- libaiscf.py: 105: what's wrong with previous implementation?
The old version was simply broken - there was an extraneous "self" argument. Any
attempt to call that function resulted in a traceback.
Thanks,
Martin
PS. test_ai_smf_service.py: can't get more optimistic than wobbly knees and
failing eyesight..
On 2/28/2011 11:44 AM, Sue Sohn wrote:
Please do a code review of the changes for:
7020631 Convert create-service.c to python
http://monaco.sfbay.sun.com/detail.jsf?cr=7020631
Webrev:
http://cr.opensolaris.org/~sohn/7020631
Thanks,
Sue
_______________________________________________
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
