Based on the comment about the localized string below, and the changes made, I'm happy with this.
Thanks, Darren. On 15/09/2011 18:56, Dermot McCluskey wrote: > Hi John, > > Yes - I ensured that I re-used existing localized strings > for the new dialog. Thanks for reviewing. > > - Dermot > > > > On 09/15/11 18:50, John Fischer wrote: >> Dermot, >> >> Still looks good. I do note that you have added a modal dialog with 2 >> localized messages >> both messages are already within the gui-installer space. So it looks good. >> >> Thanks, >> >> John >> >> On Sep 15, 2011, at 10:33 AM, Dermot McCluskey wrote: >> >>> Darren, >>> >>> Thanks for reviewing. Both suggestions accepted >>> (and tested). >>> >>> New webrev: >>> https://cr.opensolaris.org/action/browse/caiman/dermot/7090232-2/ >>> >>> Incremental since round #1: >>> https://cr.opensolaris.org/action/browse/caiman/dermot/7090232-2-incremental/ >>> >>> - Dermot >>> >>> >>> >>> On 09/15/11 17:38, Darren Kenny wrote: >>>> Hi Dermot, >>>> >>>> Generally looks fine, but I do have some comments: >>>> >>>> 659 slices = partition.get_children(class_type=Slice) >>>> ... >>>> 661 + for slc in slices: >>>> 662 + partition.delete_slice(slc) >>>> >>>> >>>> could be rewritten as simply: >>>> >>>> partition.delete_children(class_type=Slice) >>>> >>>> I don't believe there is any reason that you must call delete_slice(), is >>>> there? >>>> >>>> 679 + LOGGER.error("Internal Error: Solaris2 partition " >>>> 680 + "should have exactly 1 gap.") >>>> 681 + LOGGER.error("Gaps found: %s" % gaps) >>>> 682 + LOGGER.error("Attempting to continue anyway.") >>>> >>>> Can you really continue here? I would think you could handle> 1 gap >>>> easily >>>> enough - but I don't think continuing here without creating the slice would >>>> really work. >>>> >>>> Thanks, >>>> >>>> Darren. >>>> >>>> >>>> On 15/09/2011 17:22, Dermot McCluskey wrote: >>>>> I'm seeking b175 putback approval and two code reviewers for: >>>>> >>>>> http://monaco.sfbay.sun.com/detail.jsf?cr=7090232 >>>>> GUI installer does not create rpool on whole partition >>>>> if previous slices exists in partition >>>>> >>>>> Webrev: >>>>> https://cr.opensolaris.org/action/browse/caiman/dermot/7090232/ >>>>> >>>>> Details: >>>>> This is an oversight in gui-install: in certain cases if there's >>>>> an existing Solaris partition, and it contains slices, gui-install/ >>>>> TargetController do not wipe those slices and lay down our >>>>> own preferred layout (TextInstall does this explicitly). >>>>> If the user makes any changes to the Solaris partition (type, >>>>> size) the child slice details are lost and gui-install notices this >>>>> and re-instates slice 0 at validation time. This fix takes >>>>> advantage of this by always wiping the slices and creating >>>>> slice 0 at validation time. This results in a consistent layout >>>>> after validation, regardless of whether: >>>>> - the partition has an existing slice layout >>>>> - TargetController created a slice layout >>>>> - the slice layout has previously been removed >>>>> >>>>> Testing done: >>>>> Used format to create an "unfavorable" slice layout on the >>>>> Solaris2 partition of the target disk. Ran gui-install. >>>>> Before fix: if no changes are made to the partition, the >>>>> old slice layout is retained. >>>>> After fix: regardless of whether changes are made, the >>>>> correct slice 0 layout is imposed. >>>>> >>>>> >>>>> - Dermot >>>>> >>>>> _______________________________________________ >>>>> 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 >> _______________________________________________ >> 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 _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
