Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add swagger.yaml and generate a few types from the spec #27117

Merged
merged 7 commits into from
Oct 22, 2016

Conversation

dnephin
Copy link
Member

@dnephin dnephin commented Oct 3, 2016

Related issues: #22931, #5893, docker/engine-api#77

The swagger spec was developed here: https://github.com/bfirsh/docker-api-reference/blob/master/swagger.yaml. That repo include some validation checks which I think we should also include if this PR is merged.

The swagger spec will initially be used:

Eventually we should also be able to generate api/server/router/* and most of client/* from the spec. Most of the code in those packages is very boilerplate, and would be a lot more consistent if it were generated from a spec.

@dnephin
Copy link
Member Author

dnephin commented Oct 3, 2016

I'm going to continue to add a few types of this PR as it is being discussed.

cc @bfirsh , @cpuguy83

@dnephin
Copy link
Member Author

dnephin commented Oct 3, 2016

I guess the types could also be generated from a go:generate instruction.

@dnephin dnephin changed the title Add swagger.yaml and generate one type from the spec Add swagger.yaml and generate a few types from the spec Oct 4, 2016
@dnephin dnephin force-pushed the swagger-gen branch 2 times, most recently from 2abf33e to 3cb9d36 Compare October 6, 2016 14:39
@vieux
Copy link
Contributor

vieux commented Oct 10, 2016

Big 👍 on the proposed change.

@dnephin dnephin force-pushed the swagger-gen branch 2 times, most recently from 44ef526 to 0c12029 Compare October 17, 2016 20:31
@dnephin
Copy link
Member Author

dnephin commented Oct 17, 2016

@mlaventure the generated types are now using normal // go comments

@mlaventure
Copy link
Contributor

It looks some much cleaner, thanks!

They're not go-lint proof, but I see we added them to the skip list,

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Oct 18, 2016

design LGTM

@thaJeztah
Copy link
Member

ping @dnephin can you rebase? Then I think we should be ready to go.

We may need to describe the process (i.e. when to generate, integrate into CI etc)

Generate Volume type from the swagger.yaml
Add makefile target for generating the models

Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
and rename it to a more appropriate name ImageSummary.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
generation fixed some comments.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin
Copy link
Member Author

dnephin commented Oct 20, 2016

rebased

@dnephin
Copy link
Member Author

dnephin commented Oct 20, 2016

Yes, this also needs CI integration. It should work the same way as our vendor check (but it runs much faster).

@LK4D4
Copy link
Contributor

LK4D4 commented Oct 21, 2016

LGTM

@thaJeztah thaJeztah added this to the 1.13.0 milestone Oct 22, 2016
@thaJeztah
Copy link
Member

looks like we have enough LGTM's and CI is 💚, merging

@thaJeztah thaJeztah merged commit 3c385b9 into moby:master Oct 22, 2016
@dnephin dnephin deleted the swagger-gen branch October 22, 2016 01:16
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
Add swagger.yaml and generate a few types from the spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants