Hi Dave, Thank you for the code review. Please see my response to your comments below.
On 01/15/10 07:44 AM, Dave Miner wrote: > > ai_sparc_image.xml (and the other manifests): I'd suggest the new > parameter closer to the top, before the "rarely configured" signpost > (and probably before the package list). I have moved the root password parameter to before the package list for all the manifests. > > pre_boot_archive_pkg_image_mod.py: > > I'm bothered that we're proliferating mechanisms for setting > passwords. ICT presently uses the PasswordFile class from IPS to do > essentially the same thing (minus the encryption). I'll leave it to > you as to whether you want to reconsider this now or file a bug to > deal with it later; comments below in case you stick with this code: > Using the PasswordFile class from IPS is a great idea. I updated the code to use that instead of writing my own. > 105: Can break from loop at this point > > 120: we're not saving timezone here; seems this line should be deleted > > install_utils.py > > 770: any reason not to just use string.printable? The reason I can not use string.printable because characters from the salt should only contain characters from the set [a-zA-Z0-9./] according to the crypt_unix(5) man page. An updated webrev with the changes I made addressing your comments and Keith's comments is here: http://cr.opensolaris.org/~ktung/12726-updated/ Let me know if you have further comments. Thank you again for your review. --Karen
