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
