On 01/21/10 05:17 AM, Jan Damborsky wrote: > Hi Dave, > > thank you very much for review. > Please see my response in line. > > Jan > > > On 01/20/10 04:52 PM, Dave Miner wrote: >> On 01/19/10 08:14 AM, Jan Damborsky wrote: >>> Hi, >>> >>> could I please ask two pair of eyes to take a look at >>> fix for following bug: >>> >>> 13393 min_mem64=1536 in AI GRUB menu should be revisited >>> >>> webrev: >>> http://cr.opensolaris.org/~dambi/bug-13393/ >>> >> >> grub_setup.py: >> >> 136: seems to me the value should come from something in the manifest >> rather than being hard-coded. > > To tell the truth, I was originally considering this possibility, > then decided to go with simplified approach. > Thinking about this again, I can see that it is better to do it now > while we are in that code. I have changed the code and DC manifest + > schema accordingly. > Tested scenarios: > * DC AI x86 build done with min_mem64 commented out in the manifest > - verified that correct default value was picked up from > DC-manifest.defval.xml and saved to .image_info file > > * DC AI x86 build done with min_mem64 specified in the manifest > - verified that specified value was saved to .image_info file > > * DC AI Sparc, x86 LiveCD builds done (since DC-manifest.defval.xml > & DC-manifest.rng were touched) > >> >> 140-151: couldn't you simplify by using a with statement instead? > > To be honest, I was not aware of 'with'. I have changed the > code to take advantage of this. > > webrev has been updated accordingly: > http://cr.opensolaris.org/~dambi/bug-13393-cr/ >
Looks fine now. Dave
