On 01/02/2012 10:49, Matt Keenan wrote: > Hi Darren, > > Thanks for the review : > > On 01/30/12 18:14, Darren Kenny wrote: >> Hi Matt, >> >> Generally looks good, but I do have some small comments/nits: >> >> auto-installer.src: >> - Could we use a variable at the top of the file for the path >> of "/var/log/install" rather than direct text? > > The two mentions of /var/log/install are within echo statements but > still makes sense to have one definition as a variable so DONE > Cool
>> >> summary.txt: >> - Did you intentionally make this file executable? Seems to be set to 755 >> in the webrev. >> > > Nope, on a fresh checkout of slim_source, all the txt files for > text-install/helpfiles are set as executable by default I don't change this. > OK, just curious, I was under the impression that normally webrev doesn't show the mode of the file unless it's changed, that's why I thought it was different... Thanks, Darren. > cheers > > Matt > >> No need to another webrev if you make these changes... >> >> Thanks, >> >> Darren. >> >> On 30/01/2012 17:50, Matt Keenan wrote: >>> Hi, >>> >>> Can I get CR for following bug : >>> 7130956 - Change default log location to /var/log/install >>> http://monaco.us.oracle.com/detail.jsf?cr=7130956 >>> >>> Webrev : >>> https://cr.opensolaris.org/action/browse/caiman/mattman/7130956/ >>> >>> >>> This fix is a follow on from the recently integrated bug 7107775, and >>> turned out to be a lot less painful than envisioned, in fact rather trivial. >>> >>> Testing : >>> - Built all three ISOS ai/text/gui and test installed to ensure logs >>> are being installed to /var/log/install >>> - Ran complete set of unit tests and no regressions found. >>> _______________________________________________ >>> 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
