Dave Miner wrote: > 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?
Ah! I see your point now. I do need to close the file but not check if op == None. I'm with ya now! Thanks > > Dave
