Thanks for the review drew.
On 01/23/12 16:16, Drew Fisher wrote:
Matt,
A few nits but nothing serious:
text-install/__init__.py
316-317: The comment is all wrong here, no? You're not checking for
anything here, right?
Also, why have an entire function to do 3 lines worth of work? I'd just
inline the work right there at line 301.
Remove comment and inlined the three lines. Makes sense.
gui-install/src/__init__.py
132, 133: indent to match
191-192: comment again
Same general comment on the 3 line function here.
All done
auto-install/auto_install.py
1215-1216: comment again
Removed
1251: why the blank line?
Removed
install_ict/transfer_files.py
151-152: I don't think this check is necessary. You're controlling how
it's called and in each of the three cases, you're passing both args
167-169: You can simple use tf_dict.update(files) rather than walk the
key list.
Good catch, done.
Will rerun some new iso's with these changes and retest.
cheers
Matt
-Drew
On 1/23/12 9:00 AM, Matt Keenan wrote:
Hi,
Can I get another review for bug :
7107775 - install: log the /var/adm/messages from the installation on
the target
http://monaco.us.oracle.com/detail.jsf?cr=7107775
Webrev Located at:
https://cr.opensolaris.org/action/browse/caiman/mattman/7107775/
Fix is same as before except I've now striped out the common code from
AI, GUI and TI and placed it as a separate method that can be commonly
used by all three installers into the transfer_files ICT checkpoint
code itself.
I was going to place it as a common method in
install_common.__init__.py however the import of InstallEngine caused
a cicrular import error. Upon thinking it makes more sense to have
this contained within the transfer_files ICT code itself.
Testing:
- Built ISO's for AI, TI, GUI and test installed all three ensuring
/var/adm/messages was delivered to /var/sadm/system/logs.
Also verified that on all three installs /var/sadm/install/logs is a
symlink to /var/log/install
- Ran a complete set of caiman hg repo unit tests and verified no
regressions have occurred.
cheers
Matt
_______________________________________________
caiman-discuss mailing list
caiman-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
