Dave Miner wrote: > Tim Knitter wrote: >> Dave and Evan, >> >> I addressed both your comments in the latest webrev. Please take a look >> >> http://cr.opensolaris.org/~tsk/1868_slim/ >> > > beadm.py > > 1339 et al I'm bugged as can "be" about the inconsistent capitalization > of BE/Be in this code.
lol I did follow a convention. When using "be" in a variable name, if it came first I left it lowercase, when it didn't come first it was capitalized to keep with the style of the other variables that didn't use an acronym. I'll file a bug to use all uppercase when referencing a BE in a function or variable name. > > I'd agree with Ethan's comment about moving be_is_active_on_boot to > be_activate.c. This is why I generally despise source files called > "*utils.c", as they turn into a dumping ground of random things. It's true. > not like conserving the number of source files is an actual goal in > development... right. Thanks Tim > > Dave > > >> Thanks >> Tim >> >> Dave Miner wrote: >>> Tim Knitter wrote: >>>> >>>> Dave Miner wrote: >>>>> Tim S. Knitter wrote: >>>>>> Can someone please review the following? >>>>>> >>>>>> 1868 beadm destroy creates an empty grub menu >>>>>> >>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=1868 >>>>>> http://cr.opensolaris.org/~tsk/1868_slim/ >>>>>> >>>>> The fix seems a little problematic yet. The sequencing you've >>>>> chosen here means that if we fail to activate the current menu item >>>>> (which, though fairly unlikely, is certainly possible), then we >>>>> still end up with a GRUB menu without an active entry. I'd rather >>>>> we did things in an order that made that not possible. >>>>> >>>> I fixed this in the latest webrev. If you could verify when you can >>>> find a spare moment, I'd appreciate it. >>>> >>> Two things: >>> >>> - I found it odd that be_activate_current_be is off in the be_utils.c >>> rather than in be_activate.c. Any particular reason it's there? >>> >>> - It seems like beadm perhaps should print a message noting which be >>> will be active, just so the user realizes this and can correct if >>> desired. Or, perhaps have the confirmation prompt that's put up note >>> this case. >>> >>> Dave >
