Re: [Libguestfs] [PATCH 1/4] generator: add always-available optgroups
On Wed, May 21, 2014 at 10:47:04AM +0200, Pino Toscano wrote:
On Tuesday 20 May 2014 21:40:21 Richard W.M. Jones wrote:
> On Tue, May 20, 2014 at 07:54:45PM +0200, Pino Toscano wrote:
> > Support the possibility to have optional groups always enabled (e.g.
> > because they were present in the past, and they need to be kept for
> > users).
> > Add and use few helper optgroups-related functions to deal also with
> > them.
>
> What do you think about the attached addition to this patch?
let optgroups_names_all =
- List.sort compare (optgroups_names @ optgroups_available)
+ List.sort compare (optgroups_names @ optgroups_retired)
Wouldn't this give duplicate values if a feature appears in both
internal_optgroups_available/optgroups_retired and the auto-generated
optgroups_names?
If we only put retired optgroups into the optgroups_retired list then
it's not a problem.
Actually your comment does explain what the loop does that I removed
in my patch. I was wondering what it was for since it would have no
effect in the current code (where optgroups_retired are only retired
optgroups).
I think a better way to do this is to have the generator check -- and
die with an error -- if retired optgroups appear in the list of actions.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org