Looks good to me now, +1 Thanks,
Darren. On 17/01/2012 09:08, Jan Damborsky wrote: > Thank you for review, Darren. > > Looking at documentation (*), it is not quite clear to me if empty tuple > could be returned. > Since we don't check length of that returned tuple, I believe it is > reasonable > to catch IndexError to be on a safe side. > > I modified the code accordingly - incremental webrev is available at: > > https://cr.opensolaris.org/action/browse/caiman/dambi/cr-7118945-diff/webrev-cr-diff/ > > Jan > > > (*) http://docs.python.org/library/locale.html > > > On 01/17/12 08:46, Darren Kenny wrote: >> Hi Jan, >> >> Generally, looking good, but I do have a small question/comment: >> >> system_info.py: >> >> - lines 255-260 >> >> Is there any possibility of locale.getdefaultlocale() returning an >> empty list? If so, you should probably also catch IndexError. >> >> Thanks, >> >> Darren. >> >> On 16/01/2012 12:56, Jan Damborsky wrote: >>> Hi, >>> >>> I would appreciate code review of changes for following CRs: >>> >>> 7118945 installer unnecessarily sets NIS property config.use_broadcast >>> 7128307 [sysconfig] Traceback call when LANG variable is set to >>> non-sense value >>> >>> webrev: >>> https://cr.opensolaris.org/action/browse/caiman/dambi/cr-7118945/webrev/ >>> >>> Thank you, >>> Jan >>> >>> >>> Testing done: >>> [1] regression tests >>> >>> * built x86 AI and text install images >>> * tested text and AI installations >>> >>> [2] 7118945 >>> - non-global zone configured in interactive way with NIS >>> as naming service. Tested both 'no NIS server specified' >>> as well as 'NIS server specified' scenarios. >>> >>> - For 'no NIS server specified' scenario, verified that >>> - generated SC profile didn't configure >>> config/user_broadcast smf property >>> - ypbind(1m) was invoked in broadcast mode >>> (with -broadcast CLI option) >>> >>> [3] 7128307 >>> - deployed and configured system with invalid locale >>> specified in SC manifest >>> - verified that 'sysconfig create-profile' run on that >>> system didn't generate traceback, but instead used >>> 'C' locale as a fallback. >>> >>> _______________________________________________ >>> 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
