[libvirt] [PATCH 2/6] virsh: Create macro for "pool" option

[Peter Krempa]

On Wed, Dec 16, 2015 at 08:24:43 -0500, John Ferlan wrote:
> 
> [...]
> 
> >>  static const vshCmdOptDef opts_pool_autostart[] = {
> >> -    {.name = "pool",
> >> -     .type = VSH_OT_DATA,
> >> -     .flags = VSH_OFLAG_REQ,
> >> -     .help = N_("pool name or uuid")
> >> -    },
> >> +    OPT_POOL_COMMON

VSH_POOL_OPT_COMMON for consistency with basically all the other macros.

Also please don't include the trailing comma in the macro, it looks
weird c-wise, and will make possible regex matches for code consistency
really weird.

> >> +
> >>      {.name = "disable",
> >>       .type = VSH_OT_BOOL,
> >>       .help = N_("disable autostarting")
> > 
> > Nice. ACK
> > 
> > Should we do something similar to domain, network, ... in the rest of virsh?
> > 

Hiding stuff behind macros can be confusing sometimes, but for the most
commonly used objects it would be reasonable.

> 
> I think it would make certain things easier - I did remark on this in
> the cover letter. It gets a bit tedious to do and review; however, I
> think in the long run makes things more consistent.  There are a few
> options with "slight" differences, but nothing that cannot be worked out.

Easier? not really, it would just introduce less code when adding new
stuff at the slight cost of having to resolve the macro when looking up
what the stuff is doing. That's why only the most common objects should
be converted.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151216/500c46f9/attachment-0001.sig>