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

[Andrea Bolognani]

On Thu, Nov 09, 2023 at 08:13:38PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:59:50AM -0800, Andrea Bolognani wrote:
> > Now, I'm not sure I would go as far as suggesting that the
> > GetName() function should be completely removed, but maybe we
> > can try leaving it out from the initial version and see if
> > people start screaming?
>
> It might be useful for debugging too. I would rather log
> e.GetName() than the string version of the type but if that's the
> only reason we needed, I agree on removing for now.

I think the upside is too small considering the potential for abuse.

> > API-wise, I'm not a fan of the fact that we're forcing users to call
> > (Un)MarshalEvent instead of the standard (Un)MarshalJSON. If we add
> > something like
> >
> >   func GetEventType(data []byte) (Event, error) {
> >
> > it becomes feasible to stick with standard functions. We can of
> > course keep the (Un)MarshalEvent functions around for convenience,
> > but I don't think they should be the only available API.
>
> I agree. I'll change it. Perhaps we shouldn't use
> (Un)MarshalEvent at this layer at all. Probably the same for
> (Un)MarshalCommand.

Yeah, what I wrote for events applies 1:1 to commands as well.

Up to you whether or not you want to keep around the convenience
functions. It might indeed be fine to drop them for now and consider
reintroducing them later if it turns out that it really helps making
client code less clunky.

-- 
Andrea Bolognani / Red Hat / Virtualization