Looks good.
Sue
On 01/ 5/12 08:31 AM, Tomas Dzik wrote:
Thanks Sue, fixed.
Tomas D.
Dne 5.01.12 17:16, Sue Sohn napsal(a):
Hi Tomas,
Kind of a nit, but please capitalize known_archs and known_cpus as they
are constants.
Thanks,
Sue
On 01/ 5/12 03: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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
