Skip to content

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

Merged
thaJeztah merged 7 commits intomoby:masterfrom
dnephin:swagger-gen
Oct 22, 2016
Merged

Add swagger.yaml and generate a few types from the spec#27117
thaJeztah merged 7 commits intomoby:masterfrom
dnephin:swagger-gen

Conversation

@dnephin
Copy link
Member

@dnephin dnephin commented Oct 3, 2016

Related issues: #22931, #5893, docker-archive-public/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 <[email protected]>
and rename it to a more appropriate name ImageSummary.

Signed-off-by: Daniel Nephin <[email protected]>
generation fixed some comments.

Signed-off-by: Daniel Nephin <[email protected]>
@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