Niall: It is OK. Some of these might be taken care of in some other slim source fixes already. There is already a bug filed again pep8/pylint issues for auto_install and I will take a look again after the major projects are done.
----- Original Message ----- From: [email protected] To: [email protected] Cc: [email protected] Sent: Monday, March 19, 2012 6:15:39 PM GMT -08:00 Tijuana / Baja California Subject: Re: [caiman-discuss] Sanity check review for installadm UEFI GRUB2 changes Hi Mary, Thanks for reviewing. If you don't have any objection, I'd rather leave these alone since my goal here is to ensure consistency between the installadm specific webrev and the entire project webrev. These should be fixed in a separate diff in slim_source, to avoid any potential workspace merging complications. The slim_uefi webrev for installadm is rather prone to merging issues since it included several dozen PEP8 fixes, white space and like, that had I had my head screwed properly when reviewing, I should have asked they be removed. It has mad made merging an unnecessary headache since it makes merge failures much more common. I don't want to add this to the list of points of failure :) Thanks, Niall On 20/03/2012 12:33, Mary Ding wrote: > Niall: > > My comments are nits. pylint had been complaining about some of these > warnings/errors and it will be nice if those can be fixed: > > 1. > http://jurassic.us.oracle.com/~npower/webrevs/webrev-slim-uefi-2012-03-19/usr/src/cmd/installadm/create_service.py.html > > > > line #42 should be removed as pkg.client.history is already imported > from the following line #39 > > import osol_install.auto_install.installadm_common as com > > 2. > http://jurassic.us.oracle.com/~npower/webrevs/webrev-slim-uefi-2012-03-19/usr/src/cmd/installadm/service_config.py.html > > line #322: > > logging.log(com.XDEBUG, "aliases=%s" % aliases) > > It probably should be: > > logging.log(com.XDEBUG, "aliases=%s" aliases) > > > > > On 03/19/12 15:37, Niall Power wrote: >> Hi, >> >> In addition to distro constructor, I aim to integrate the installadm >> changes into build 13, before the final UEFI/GRUB2 feature push in >> build 14. >> This will ease the transition for admins and reduce flag day >> severity, since an AI server upgraded to build 13 will be able to >> provision build 14 (GRUB2 based) images and service UEFI clients. >> >> >> So, this webrev is a strickt subset of the overall UEFI project >> webrev and has already been code reviewed. The main purpose of this >> request is to ensure that the right pieces have been taken from the >> main webrev and that nothing is missing to satisfy installadm. >> >> I have built and sanity tested this subset webrev to ensure that it: >> - can import, provision a legacy GRUB based AI ISO image, and that >> the client can boot from it. >> - can import, provision a GRUB2/UEFI based image, that it creates >> bios and uefi dhcp entries, and that the client can boot from it. >> >> installadm specific webrev: >> http://jurassic.us.oracle.com/~npower/webrevs/webrev-installadm-uefi-2012-03-19/ >> >> >> >> original full project webrev: >> http://jurassic.us.oracle.com/~npower/webrevs/webrev-slim-uefi-2012-03-19/ >> >> >> Thanks! >> Niall >> _______________________________________________ >> 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
