You're correct - os.path.join is not the behavior I wanted. I'm going to revert back to plain old string concatenation. Thanks for pointing that out!
- Keith On 02/12/10 09:51 AM, Ethan Quach wrote: > Keith, > > ti_install.py. - 521 > > Just make sure "install_profile.log_final" is *not* an absolute path > otherwise os.path.join() does something stupid IMO; it'll just take > the second arg as the "joined" path. > > > -ethan > > > On 02/12/10 09:46, Keith Mitchell wrote: >> Hi John, >> >> Thanks for the review! An updated webrev is here: >> http://cr.opensolaris.org/~kemitche/14180b >> >> - Keith >> >> On 02/11/10 05:16 PM, John Fischer wrote: >>> Keith, >>> >>> A couple of issues: >>> >>> line 194 -- raises ti_utils.InstallationErrorer >>> probably should be ti_utils.InstallationError >>> like others >> >> Indeed it should - good catch. >> >>> >>> lines 482 - 487 -- uses a finally. Is there any possibility >>> that an exception will be raised from the run_ICTs() >>> and lost if finally is run? Or am I misunderstanding >>> the Python 2.6 documentation (which very well could be the case)? >> >> If an exception is raised in the "try" clause, the "finally" block will >> get run. After the finally block executes, the original exception will >> "take over" again - it won't be lost. >> >>> >>> line 515 -- why not use os.path.join() instead of using + >>> to create a path >>> >>> >>> Nits: >>> >>> The comments for the class and function introductions >>> could be a little more verbose. >> >> I did a passthrough and updated some of the comments. >> >>> >>> The comments at lines 493-496 do not seem to match the other >>> function comments stylistically. >>> >>> 493 '''Do final cleanup to prep system for first boot, such as >>> resetting >>> 494 the ZFS dataset mountpoints >>> 495 >>> 496 ''' >>> >>> verses: >>> >>> 314 '''Call libtransfer to transfer the bits to the system via cpio.''' >> >> That's actually PEP8 compliant - multi-line docstrings have a blank line >> in between the last line of the comment and the closing quotes, whereas >> single-line docstrings are constrained to a single line. >> >>> >>> >>> Interesting/handy: >>> >>> " ".join(cmd) >>> >>> Thanks, >>> >>> John >>> >>> On 02/11/10 03:41 PM, Keith Mitchell wrote: >>>> Hi all, >>>> >>>> I'd like to request a code review of my changes for bug 14180. >>>> >>>> Bugzilla link: http://defect.opensolaris.org/bz/show_bug.cgi?id=14180 >>>> >>>> webrev: http://cr.opensolaris.org/~kemitche/14180/ >>>> >>>> I tested the changes by using a modified keyboard_layout ICT that >>>> always >>>> failed, and verified that the installer cleans up properly upon >>>> completion. >>>> >>>> This webrev also includes a minor change to the README in the text >>>> installer gate; this issue has no associated bugzilla bug, but I can >>>> file one if needed. >>>> >>>> - Keith >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
