Libguestfs - May 2014 - Re: [PATCH 1/4] generator: add always-available optgroups

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?

(It is meant to be squashed into your patch.)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

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?

-- 
Pino Toscano

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