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
