Hi,

On Mon, Oct 02, 2023 at 05:48:37PM -0400, John Snow wrote:
> On Fri, Sep 29, 2023 at 8:37 AM Daniel P. Berrangé <berra...@redhat.com> 
> wrote:
> >
> > On Fri, Sep 29, 2023 at 02:23:22PM +0200, Victor Toso wrote:
> > > Hi,
> > >
> > > On Thu, Sep 28, 2023 at 03:51:50PM +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Sep 27, 2023 at 01:25:37PM +0200, Victor Toso wrote:
> > > > > This patch handles QAPI alternate types and generates data structures
> > > > > in Go that handles it.
> > > >
> > > > This file (and most others) needs some imports added.
> > > > I found the following to be required in order to
> > > > actually compile this:
> > >
> > > This was by design, I mean, my preference. I decided that the
> > > generator should output correct but not necessarly
> > > formatted/buildable Go code. The consumer should still use
> > > gofmt/goimports.
> > >
> > > Do you think we should do this in QEMU? What about extra
> > > dependencies in QEMU with go binaries?
> >
> > IMHO the generator should be omitting well formatted and buildable
> > code, otherwise we need to wrap the generator in a second generator
> > to do the extra missing bits.
> >
> 
> Unless there's some consideration I'm unaware of, I think I agree with
> Dan here - I don't *think* there's a reason to need to do this in two
> layers, unless there's some tool that magically fixes/handles
> dependencies that you want to leverage as part of the pipeline here.

But there is. In the current qapi-go repository, I have 4 line
doc.go [0] file:

 1  //go:generate ../../scripts/generate.sh
 2  //go:generate gofmt -w .
 3  //go:generate goimports -w .
 4  package qapi

With this, anyone can run `go generate` which runs this generator
(1), runs gofmt (2) and goimports (3).

- gofmt fixes the formatting
- goimports adds the imports that are missing

Both things were mentioned by Dan as a problem to be fixed but
somewhat can be solved by another tool.

From what I can see, we have three options to chose:

 1. Let this be as is. That means that someone else validates
    and fixes the generator's output.

 2. Fix the indentation and the (very few) imports that are
    needed. This means gofmt and goimports should do 0 changes,
    so they would not be needed.

 3. Add a post-processing that runs gofmt and goimports from
    QEMU. I would like to avoid this just to no add go binaries
    as requirement for QEMU, but perhaps it isn't a big deal.

I'm planning to work on (2) for v2 and further discuss if (3)
will be needed on top of that.

[0] https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/doc.go

Cheers,
Victor

Attachment: signature.asc
Description: PGP signature

Reply via email to