Hi Markus,

On Mon, Jun 27, 2022 at 09:15:53AM +0200, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
> 
> > Hi,
> >
> > This is the second iteration of RFC v1:
> >   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
> >
> >
> > # What this is about?
> >
> > To generate a simple Golang interface that could communicate with QEMU
> > over QMP. The Go code that is generated is meant to be used as the bare
> > bones to exchange QMP messages.
> >
> > The goal is to have this as a Go module in QEMU gitlab namespace,
> > similar to what have been done to pyhon-qemu-qmp
> >   https://gitlab.com/qemu-project/python-qemu-qmp
> 
> Aspects of review:
> 
> (1) Impact on common code, if any
> 
>     I care, because any messes made there are likely to affect me down
>     the road.

For the first version of the Go generated interface, my goal is
to have something that works and can be considered alpha to other
Go projects.

With this first version, I don't want to bring huge changes to
the python library or to the QAPI spec and its usage in QEMU.
Unless someone finds that a necessity.

So I hope (1) is simple :)

> (2) The generated Go code
> 
>     Is it (close to) what we want long term?  If not, is it good enough
>     short term, and how could we make necessary improvements?
> 
>     I'd prefer to leave this to folks who actually know their Go.
> (3) General Python sanity
> 
>     We need eyes, but not necessarily mine.  Any takers?
> 
> [...]
> 
> >  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
> >  scripts/qapi/main.py   |   2 +
> >  2 files changed, 767 insertions(+)
> >  create mode 100644 scripts/qapi/golang.py
> 
> This adds a new generator and calls it from generate(), i.e.
> review aspect (1) is empty.  "Empty" is a quick & easy way to
> get my ACK!
> 
> No tests?

I've added tests but on the qapi-go module, those are the files
with _test.go prefix on them. Example for commands:

    https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/commands_test.go

Should the generator itself have tests or offloading that to the
qapi-go seems reasonable?

> No documentation?

Yes, this iteration removed the documentation of the generated
types. I'm a bit sad about that. I want to consume the
documentation in the QAPI files to provide the latest info from
types/fields but we can't 'copy' it, the only solution is 'refer'
to it with hyperlink, which I haven't done yet.

Cheers,
Victor