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. 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 >
