On 3/2/10 9:40 AM, Keith Mitchell wrote: > Hi Evan, > > Most of it looks good. Just some minor things: > > 2284: Catch IOError as well. Since you're not making use of "errno", > then the line: > "except (OSError, IOError) as err:" > would be appropriate (replacing "strerror" with "str(err)")
Fixed. > > 2283, 2286: I'm never sure if webrev shows alignments properly (as it > doesn't handle whitespace) - make sure that these lines are aligned just > after the opening parentheses from the previous line to be PEP8 > compliant. (The trailing slashes on the previous lines are also > technically not needed as they're inside parentheses) Fixed both the indentation and removed the unneeded \ > > 2283 vs 2286: In the shutil.copy2 call, you use self.rootpool - in the > error message, you reference self.basedir - which is correct? The error message was incorrect. I've updated the webrev with these changes. Thanks for taking a look! -evan > > Thanks, > Keith > > On 03/ 1/10 09:13 PM, Evan Layton wrote: >> I need to get a review for bug 14962. >> >> Bug: >> http://defect.opensolaris.org/bz/show_bug.cgi?id=14962 >> >> Webrev: >> http://cr.opensolaris.org/~evanl/14962/ >> >> Some background: >> This bug resolves the issue related to installing in a logical >> partition where the first time a BE is activated the activate >> will fail due to running installgrub when it doesn't need to. >> >> This does not resolve the issues related to bug 14894. >> >> Thanks, >> -evan >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
