Karen Tung wrote: > Hi Jean, > > Thank you for making the change. I have one more comment about the code. > > I think it is better to just get rid of lines 61-63. Then, in line > 583, just assign rv to be 2. > That way, the "finally" clause will take care of the shutting down of > the logging system. > The work of logging shutdown will be done in only 1 spot in the whole > DC app. In case we need > to change how logging is shutdown in the future, we don't need to > worry about modifying > all the different places. Done.
Jean > > Thanks, > > --Karen > > Jean McCormack wrote: >> I've updated the webrev with Karen's suggestions. >> http://cr.opensolaris.org/~jeanm/slim_4352_4354/ >> >> Jean >> >> >> Karen Tung wrote: >>> Jean McCormack wrote: >>>> Please review the following webrev for defects 4352 & 4354 >>>> >>>> Defects: >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4352 >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4354 >>>> >>>> Webrev: >>>> http://cr.opensolaris.org/~jeanm/slim_4352_4354/ >>>> >>>> Jean >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>> Hi Jean, >>> >>> Here are my comments: >>> >>> distro_const.py, line 51: >>> - What's changed in this line? I don't see anything, but it >>> indicated that it's changed. >>> >>> distro_const.py, line 574: >>> - While the code works the way it is now, >>> I am concern about the possibility of information lost by just >>> returning 2 for >>> all the SystemExit. It's true that right now, the usage() is the >>> only thing that returns 2. >>> What about in the future? What about other part of the DC code that >>> might >>> call SystemExit() with an error message that we want to see? >>> IMO, it is more correct to define an exception specific to usage >>> related errors, >>> and catch that exception, assign the appropriate return code, and >>> leave SystemExit() >>> the way it is. >>> >>> Thanks, >>> >>> --Karen >> >
