[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] generator: Add visibility to action struct



On Tue, 2013-02-05 at 14:40 +0000, Richard W.M. Jones wrote:
> On Mon, Feb 04, 2013 at 02:18:48PM +0000, Matthew Booth wrote:
> > +let is_external =
> > +  fun x -> match x.visibility with
> > +  | VPublic | VStateTest | VBindTest | VDebug -> true
> > +  | VInternal -> false
> 
> I know we discussed this on IRC, but seeing the whole
> function I now realise you could write:
> 
>   let is_external { visibility = v } =
>     match v with
>     | VPublic | VStateTest | VBindTest | VDebug -> true
>     | VInternal -> false
> 
> Or:
> 
>   let is_external = function
>     { visibility = (VPublic | VStateTest | VBindTest | VDebug) } -> true
>     { visibility = VInternal } -> false
> 
> Whether you prefer this or not is up to you :-)

Changed.

> Anyway, this all looks a lot more sensible.
> 
> One check you could do is to diff the generator output before
> and after the change.
> 
> Basically, reset to before your patch, and do:
> 
>   make -C generator
>   for f in `cat generator/files-generated.txt`; do cp $f $f.orig; done
> 
> Then apply your patch, and do:
> 
>   make -C generator
>   for f in `cat generator/files-generated.txt`; do diff -u $f.orig $f; done \
>     | less

This highlighted that I'd unintentionally moved bindtests from private
to public in guestfs.h. Apart from that it's all good.

I've fixed the above and pushed.

Thanks,

Matt


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]