Conversation
Signed-off-by: Daniel Nephin <[email protected]>
Signed-off-by: Daniel Nephin <[email protected]>
Signed-off-by: Daniel Nephin <[email protected]>
Signed-off-by: Daniel Nephin <[email protected]>
10ff43a to
0919c34
Compare
|
It looks like yamllint was removed as part of the multi-stage Dockerfile PR, but validation never run because our validation checks for changes before running. I'll look at restoring yamllint. |
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks! Left some thoughts 👍
hack/validate/gometalinter.json
Outdated
| ".*\\.pb\\.go", | ||
| "dockerversion/version_autogen.go", | ||
| "api/types/container/container_.*", | ||
| "api/types/.*-generated\.go", |
There was a problem hiding this comment.
Should this have double backslashes?
|
|
||
| The `Status` field is optional, and is omitted if the volume driver | ||
| does not support this feature. | ||
| */ |
There was a problem hiding this comment.
Not a big fan of the block-style comments; seems they're generated by this https://github.com/dave/jennifer/blob/master/jen/comments.go#L85-L93, so not sure there's an easy workaround (other than splitting on newlines and appending a single-line comment for each line)
There was a problem hiding this comment.
Yeah if there's a newline in a comment, jennifer will use multi-line syntax. However, if you prefix your comment with // it'll be rendered as-is. You'd need to manually add the // to the start of every line though. Feel free to ping me if you'd like any more jennifer help!
| } | ||
|
|
||
| // OK response to ContainerWait operation | ||
| type ContainerWaitResponse struct { |
There was a problem hiding this comment.
Wondering what determines if this is named ContainerWaitOKResponse vs ContainerWaitResponse.
Although I like "response"; "body" may be more factual (i.e. it's not the full response); do you know where the code is to generate the names?
Basically; should we try to make the names match the old names?
There was a problem hiding this comment.
Comment doesn't look to be correct godoc as well
| cd "$GOPATH/src/github.com/dnephin/swagger-gen" | ||
| git checkout -q "$SWAGGER_GEN_COMMIT" | ||
|
|
||
| go build -o ${PREFIX}/swagger-gen . |
There was a problem hiding this comment.
nit: need to replace spaces with a tab
| COPY --from=swagger /usr/local/bin/swagger* /usr/local/bin/ | ||
| COPY --from=frozen-images /docker-frozen-images /docker-frozen-images | ||
| COPY --from=gometalinter /opt/gometalinter/ /usr/local/bin/ | ||
| COPY --from=swaggergen /opt/swaggergen/ /usr/local/bin/ |
There was a problem hiding this comment.
Should this hunk belong to the first commit in the series?
|
@dnephin this one needs to be rebased |
|
This hasn't been touched in a long time and no one seems to be taking it up, so I'm going to go ahead and close this. |
This PR replaces the tool used for
api/typescode generate. We have been usinggo-swaggerbut this has not worked well:go-swaggerremoved support for generating only types.go-swaggerused templates templates to generate code, which proved to be difficult to work with, and it never gave us full control over the templates.go-swaggerdefaults required us to add a lot ofx-nullable: falseall over the spec and often ignoredTitlewhen it was set.The new tool is currently at https://github.com/dnephin/swagger-gen-types, but I would like to move it to the moby organization on github, since it's primarily only going to be used with moby/moby.
The rules for generating types are simple, and the code is only ~300 lines. It uses the https://github.com/go-openapi for parsing the swagger spec, and https://godoc.org/github.com/dave/jennifer/jen for code generation.
This new tool should make it a lot easier to convert more types to code generation, and makes it a lot easier to customize the code that it generated.