Hi Sundar, thank you very much for your comments. Please see my response in line. The webrev has been updated with changes.
Jan On 04/03/09 18:58, Sundar Yamunachari wrote: > Jan, > * > usr/src/cmd/auto-install/auto_install.c* > > Can you change enable_debug_mode() to set_debug_mode() and > is_debug_mode_enabled() to get_debug_mode()? This will make it more > clear and if want to allow setting more than one debug value in the > future, it will be simpler. As far as support for more debug levels is concerned, I agree with Dave. As implemented right now, there is no common design for supporting more debug levels right now across installer technologies (e.g. beadm module has only one level). Also, as we have experienced so far, once we are interested in debug messages (e.g. when evaluating some issue), we have always asked for turning on the most verbose level in order to obtain as much information as possible. Setting intermediate levels has not been used so far. Based on current state of things, it seems unlikely that we might need that feature in near future. Please let me know what you think. > > 1582: Add a break after this line. What about having a default case to > display usage and exit if the user specifies any other option? Good catch ! I have added break and usage message is displayed if incorrect options are provided. > > 1615-1626: Needed only if debug_is_enabled. Can you move it with in > the block 1597-1613? We need to call ls_init() for initializing logging service even if debug mode is not enabled - in that case it is invoked will NULL parameter. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090406/def3a9b9/attachment.html>
