Re: [PATCH v2 04/11] qapi: golang: Generate qapi's alternate types in Go

[Victor Toso]

Hi,

On Fri, Nov 10, 2023 at 01:58:20AM -0800, Andrea Bolognani wrote:
> On Thu, Nov 09, 2023 at 08:01:48PM +0100, Victor Toso wrote:
> > On Thu, Nov 09, 2023 at 09:34:56AM -0800, Andrea Bolognani wrote:
> > > We should call this Null instead of IsNull, and also make it a
> > > pointer similarly to what I just suggested for union branches
> > > that don't have additional data attached to them.
> >
> > I don't have a strong argument here against what you suggest, I
> > just think that the usage of bool is simpler.
> >
> > The test I have here [0] would have code changing to something
> > like:
> >
> > When is null:
> >
> >   - val := &StrOrNull{IsNull: true}
> >   + myNull := JSONNull{}
> >   + val := &StrOrNull{Null: &myNull}
> >
> > So you need a instance to get a pointer.
> >
> > When is absent (no fields set), no changes as both bool defaults
> > to false and pointer to nil.
> >
> > The code handling this would change from:
> >
> >   - if u.IsNull {
> >   + if u.Null != nil {
> >         ...
> >     }
> >
> > We don't have the same issues as Union, under the hood we Marshal
> > to/Unmarshal from "null" and that would not change.
> >
> > [0] 
> > https://gitlab.com/victortoso/qapi-go/-/blob/main/test/type_or_null_test.go
> >
> > I can change this in the next iteration.
> 
> No, leave the type alone. But I still think the name should probably
> be Null instead of IsNull.

Ok, keeping bool, rename to Null. Deal.

Cheers,
Victor

signature.asc
Description: PGP signature