On Thu, Dec 08, 2011 at 11:45:18AM +0100, Agata Murawska wrote:
> On Thu, Dec 8, 2011 at 11:29 AM, Iustin Pop <[email protected]> wrote:
> > On Thu, Dec 08, 2011 at 11:26:45AM +0100, Agata Murawska wrote:
> >> Also, since both cluster init and modify use options for instance
> >> policy, I extracted them as a separate list.
> >
> > LGTM, only one tiny comment:
> >
> >> +instance_policy_opts = [
> >> +  SPECS_CPU_COUNT_OPT,
> >> +  SPECS_DISK_COUNT_OPT,
> >> +  SPECS_DISK_SIZE_OPT,
> >> +  SPECS_MEM_SIZE_OPT,
> >> +  SPECS_NIC_COUNT_OPT,
> >> +  ]
> >>
> >>  commands = {
> >>    "init": (
> >> @@ -1403,8 +1410,7 @@ commands = {
> >>       MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, DRBD_HELPER_OPT,
> >> NODRBD_STORAGE_OPT,
> >>       DEFAULT_IALLOCATOR_OPT, PRIMARY_IP_VERSION_OPT, 
> >> PREALLOC_WIPE_DISKS_OPT,
> >>       NODE_PARAMS_OPT, GLOBAL_SHARED_FILEDIR_OPT, USE_EXTERNAL_MIP_SCRIPT,
> >> -     DISK_PARAMS_OPT, MEM_COUNT_SPECS_OPT, CPU_COUNT_SPECS_OPT,
> >> -     DISK_COUNT_SPECS_OPT, DISK_SIZE_SPECS_OPT, NIC_COUNT_SPECS_OPT],
> >> +     DISK_PARAMS_OPT] + instance_policy_opts,
> >
> > Since instance_policy_opts is kind of a constant here, I kindly ask you
> > to uppercase it.
> Ack (I used lowercase because the same was done in gnt_instance with add_opts)
> 
> >
> > thanks,
> > iustin
> 
> interdiff:

LGTM (no need for interdiff on such small changes).

iustin

Reply via email to