On Tue, Jul 06, 2010 at 07:38:12PM +0100, Luca Bigliardi wrote: > On Tue, Jul 6, 2010 at 7:08 PM, Iustin Pop <[email protected]> wrote: > > > + if options.reboot_types is None: > > + options.reboot_types = constants.REBOOT_TYPES > > + else: > > + options.reboot_types = options.reboot_types.split(",") > > + rt_diff = > > set(options.reboot_types).difference(constants.REBOOT_TYPES) > > + if rt_diff: > > + Err("Invalid reboot types specified: %s" % > > utils.CommaJoin(rt_diff)) > > + > > Optionally you can also warn the user if do_reboot is None and > reboot_types is not None: > to me it seems do_reboot will silently override reboot_types.
I'm not worried about this case, as I don't want the user to pass two flags to disable reboot - as in, do_reboot is the master switch for reboot on/off, and it takes precedence over the reboot types. But I am worried about do_reboot being true and reboot_types empty, in which case we will indeed skip reboots. > Apart from this LGTM. I'll take your LGTM for now (as I need this to fix the QA runs), and the enhancements on the user part can be done as part of a later patch (if someone cares about it). iustin
