Tomas, Looks great.
Thanks, JOhn On Jan 5, 2012, at 3:37 AM, Tomas Dzik wrote: > Hi John and others, > I modified sources as you suggested and added test cases. (I also added some > test cases for similar functions.) Webrev i here: > > https://cr.opensolaris.org/action/browse/caiman/t.dzik/7015427-2/ > > Please let me know whether it is OK now. > > Regards, > > Tomas D. > > Dne 3.01.12 17:46, John Fischer napsal(a): >> Tom, >> >> This looks good in general. >> >> I think that we need to throw an exception if the Architecture and CPU >> are not supported. >> The exception can be caught where the checkCPU and checkArch are called >> if we want >> to just pass on the error. But if the exception is added then when we >> add a new version >> for either of those then the osol_install.auto_install.installadm_common >> will also be updated. >> Thus this part of the code will not need to be updated anyhow. >> >> The wording looks fine. >> >> Also what sort of test enhancements need to be made? >> >> Thanks, >> >> John >> >> >> On Jan 3, 2012, at 7:43 AM, Tomas Dzik wrote: >> >>> Hi all, >>> I would like to ask you for a code review for a bug >>> >>> 7015427 - installadm does not validate criteria values >>> >>> Webrev: >>> >>> https://cr.opensolaris.org/action/browse/caiman/t.dzik/7015427/ >>> >>> Couple of questions: >>> >>> 1) Could you please check the wording of Warnings I added ? >>> >>> 2) Is it OK just to print warnings or should I rise the exception ? >>> I decided just to print warning because in such case my changes will >>> not prevent installation of the potential new architectures but I can >>> change it if you feel that Error is more appropriate here. >>> >>> Testing: >>> >>> 1) Sources are pep8 clean. >>> 2) Tests passed >>> 3) I tried to create manifest and profile with criteria cpu and arch, >>> using allowed and not-allowed cpus and archs, using single value and >>> lists of value and I checked that proper warnings are printed and >>> criteria updated. >>> 4) I tried also installadm set-criteria command. >>> 5) I tried to set criteria using also .xml file >>> >>> Best regards, >>> >>> Tomas D. >>> _______________________________________________ >>> 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
