Jan, The changes look good now.
- Sundar jan damborsky wrote: > 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/e34b0936/attachment.html>