Hi Clay,
The changes look good. Sorry for the delay in responding!
- Keith
On 04/16/10 01:29 PM, [email protected] wrote:
Hi Keith,
Thank you for your comments. My responses below:
create_client.py:
202, 206, 211 - I think it might be more consistent if perhaps the
first line of setup_dhcp was "arch = arch.lower()", and the
comparisons checked against the lower case value. Then, the issue of
case sensitivity is moot, regardless of whether Sparc, SPARC, or
sparc is passed in. (All unlikely, but since
/usr/lib/installadm/setup-dhcp appears to require lower-case anyway,
this might be smoother).
The AIImage class should provide the architecture passed around and
the docstring does specify what's expected. However, for robustness'
sake I have added your suggestion
installadm_common:
Please put the successful run_cmd doctest first (Normal usage
scenario before the error cases).
Good idea, they're of more interest to someone understanding the API
probably.
1134, 1154: The fact that these lines are needed implies we have an
issue with how we define the "_" function in our modules (which is to
say, we don't). This is likely a larger localization issue, but I
believe importable modules are not supposed to rely on
gettext.install() having been called at some point.
This is certainly a larger issue needing a separate bug.
In any case, would a single "gettext.install()" call be sufficient?
(Or even a slightly more cryptic one-liner such as "_ = lambda x: x"?)
Yes, I think for the docstrings, it would be better to pass through
the gettext functions. Another gettext related bug was pointed out by
Shawn Walker:
14918 - AI subcommands need to guard against bogus locales
The IPS folks have a similar bug to your request as well:
6530 - _ fallback needed if gettext not present / installed
1144-1147 - As the doctest shows up in documentation, I think it's
best to limit output to the minimum necessary to make the point. With
that in mind, simply dumping test['err'] (and possibly test['out'])
here would reduce the "noise" level. (Similar thoughts can be applied
to the other examples as well - this also could have the side effect
of minimizing or eliminating the need for ELLIPSIS and similar
statements).
I've cleaned up the doctest material a bit but want to ensure that
folks have examples of the return symantics of the function.
New webrev:
http://cr.opensolaris.org/~clayb/15531/webrev2/
Differential webrev:
http://cr.opensolaris.org/~clayb/15531/webrev2.diff/
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
