On 02/ 8/10 01:20 PM, Dave Miner wrote: > On 02/ 4/10 06:00 PM, Susan Sohn wrote: >> The incremental webrev for the text installer project has been posted >> at: >> http://cr.opensolaris.org/~kemitche/text_v3_incremental/ >> >> This webrev shows the diffs from the original code review with certain >> exceptions as described in the NOTE below. >> >> The full webrev to date is located at: >> http://cr.opensolaris.org/~kemitche/text_v3/ >> >> We would like to get feedback on this round of changes no later than COB >> Tuesday, 2/9, if at all possible. If you would like to review these >> changes but >> can't be done with your comments by then please contact me and we'll >> work >> something out. >> > > Re-reviewed same set as previously, a few minor issues: > > install-finish, 129: Why is it necessary to restrict this particular > function to only the textinstall case? Granted, you're the only user > of it right now, but it seems excessively restrictive.
I spoke with Karen and there's no particular reason for this restriction, so I'll remove it. > > text-mode-menu.ksh, 49: The comment says LC_ALL, but you're in fact > setting the individual elements, as well as LANG. One or the other is > incorrect, and LANG would seem to be unnecessary, period. I'll update the comment. Setting LC_ALL is the wrong choice, as it overrides all the other LC_* variables (which has implications for a user dropping to the shell to run other utilities - admittedly not an '80% case,' but that's the reason). Setting LANG, and LANG alone, is what I believe to be the correct choice. LANG is the variable that's checked when the relevant LC_ variable is not set. As I understand it, for example, if a program is doing numerical formatting and needs to know whether to use a comma or a period as the decimal point, first LC_ALL is checked; if that's not set, then LC_NUMERIC is checked; if that's not set, then LANG is checked; if that's not set, then the system default is used.[1] If that's the case, then for a service like this that is providing a 'starting point', only LANG needs to be set. [1]http://www.opengroup.org/onlinepubs/007908799/xbd/envvar.html (See section "Internationalisation Variables") > > usr/src/lib/Makefile, 29-46: Can we re-alphabetize the list here? > Order is set by the dependencies starting at 80, so this would be > better listed alphabetically. Done. - Keith > > Dave > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
