Joseph J VLcek wrote: > Thanks Dave. > > > > Dave Miner wrote: >> Joseph J VLcek wrote: >>> Hello, >>> >>> Can two people please do a code review for bug: >>> >>> 4673 Grub text only mode is required >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4673 >>> >>> >>> The webrev is available at: >>> http://cr.opensolaris.org/~joev/bug4673/ >>> >> General comment: bootadm delimits individual entries with the "ADDED BY >> BOOTADM" / "END BOOTADM" comment pairs. You should just add the new >> entry after that, not encapsulated with the bootadm markers. > > > OK. > > > >> 948,1025: don't need multiple levels of try > > > I had checked with Jack S. on this he told me that to use finally with > Python v2.4 the extra level is needed. I get errors without it. > > > Using my standalone test program without the extra try level I get: > > opensolaris > ./modify_grub.py > File "./modify_grub.py", line 132 > finally: > ^ > SyntaxError: invalid syntax > > > Jack sent me this example: > ------------------------- > python 2.4 > > try: > try: > do something risky > except Exception: > handle the consequences > finally: > always do this > > > > python 2.5: > > try: > do something risky > except Exception: > handle the consequences > finally: > always do this >
Checked back in the docs, I guess that's right. Bizarre. > > > > >> 1013: replace loop with just all.extend(one_entry) > > > Don't want to sound nerdy but "that's very cool ;)" thanks! > >> 1030: writing, not reading > > oops... > > >> 1040: doesn't appear you can get here with op == None, as you return at >> 1023. > > > Line 1023 executes only if the open for write fails. > > Line 1040 is executed if the open succeeds. > Which is my point; how can it be None if you get to 1040, since you only get there if it succeeded? Dave
