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

Automatically generate Engine API documentation #606

Merged
merged 3 commits into from
Dec 14, 2016

Conversation

bfirsh
Copy link
Contributor

@bfirsh bfirsh commented Nov 16, 2016

I have replaced the Engine API docs with a version that is automatically generated from a Swagger definition, using ReDoc. It looks a bit like this:

screenshot 2016-11-16 18 11 40

I have also reorganised the menu for the Engine API to make it simpler and clearer:

screenshot 2016-11-16 21 29 18

Before, it was under "reference", even though some of it wasn't reference material, and it also seemed quite hidden that deep in the menu tree.

I've done a few other things, too:

Known issues

  • ReDoc is a client-side app that takes a bit of time to load. The author is working on a fix and I think the downside of this outweighs the benefit of much better documentation.

Related issues

/cc @dnephin @johndmulhausen

@johndmulhausen
Copy link

johndmulhausen commented Nov 16, 2016

Deploy preview ready!

Built with commit 007d0a5

https://deploy-preview-606--docker-docs.netlify.com

@sanscontext
Copy link
Contributor

Noting an offline discussion: the accounts apis touch more than Hub, currently in discussion about where they ought to go. :)

@johndmulhausen
Copy link

@sanscontext @mstanleyjones @londoncalling @joaofnfernandes

The most relevant staging URL is here:
https://deploy-preview-606--docker-docs.netlify.com/engine/api/v1.25/

Be aware that this PR also changes "Remote API" to "Engine API" so make sure any find/replaces were done right

@johndmulhausen
Copy link

johndmulhausen commented Nov 17, 2016

@bfirsh Can you make the links to API versions relative instead of https://docs.docker.com ? E.g. have the URLs just be /engine/api/v1.24/ with no domain. That way they'll show the version that is shipping with that branch/fork/clone of the docs

https://deploy-preview-606--docker-docs.netlify.com/engine/api/v1.25/#section/Versioning

Clicking on these now, you can see that they 404

@johndmulhausen
Copy link

Other than that, this LGTM!

@sanscontext
Copy link
Contributor

Can we hold off on merge until I've got a Docker ID stub in place to rehome the Hub docs?

@sanscontext
Copy link
Contributor

Please hold off until #634 has merged.

@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 18, 2016

@johndmulhausen Okay. That was intentional so the swagger.yaml would work standalone, but I guess it's not the end of the world making the intro description just work inside the documentation.

@johndmulhausen
Copy link

@bfirsh Belay my comment then; I get it.

@johndmulhausen
Copy link

johndmulhausen commented Nov 18, 2016

@bfirsh PR #634 has been merged and it makes sense to have the Accounts API reference live under /docker-id/ -- can you update this PR?

I suggest /docker-id/api-reference/

@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 21, 2016

Due to #511, we're now going to have to wait for moby/moby#28506 to make its way into the 1.12 branch. I will push this along. :)

bfirsh added a commit to bfirsh/docker that referenced this pull request Nov 22, 2016
See: docker/docs#606

Also:
- Add missing redirects to API reference pages
- Remove v1.25 and 1.26, because they are being replaced with
  swagger generated docs.
- Remove all other docs which aren't reference material, because
  this can live in docker/docker.github.io

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
vieux pushed a commit to vieux/docker that referenced this pull request Nov 23, 2016
See: docker/docs#606

Also:
- Add missing redirects to API reference pages
- Remove v1.25 and 1.26, because they are being replaced with
  swagger generated docs.
- Remove all other docs which aren't reference material, because
  this can live in docker/docker.github.io

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
(cherry picked from commit 993854f)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 23, 2016

moby/moby#28506 is now merged. 🎉

I'll wait for #674 before updating this PR.

@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 24, 2016

@mstanleyjones @johndmulhausen This is also just for Docker 1.13, so presumably I should open it against the branch vnext-engine? Will #511 be cherry-picked into that branch?

@bfirsh bfirsh force-pushed the swagger-api-docs branch 2 times, most recently from b561355 to 4843cf9 Compare November 25, 2016 13:05
@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 25, 2016

Okay, I have now implemented this PR on top of #511 and #674. The implementation is ready for review, apart from the hacks in Dockerfile to pull from the correct Docker version.

Once #511 and #674 have made their way into the vnext-engine branch, I can switch this PR to that branch and it should be ready to merge.

bfirsh added a commit to bfirsh/docker that referenced this pull request Nov 28, 2016
This makes the swagger.yaml useful outside of the documentation.
For background:
docker/docs#606 (comment)

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
@johndmulhausen
Copy link

@bfirsh Everything in master will be merged into vnext-engine so yes, #511 and every other fix that's happened here will be pulled in. :) As for which branch to open this PR against, yes -- if it's for v1.13 only, you want to do that against vnext-engine. Which means you might have to create a new PR for this.

@bfirsh
Copy link
Contributor Author

bfirsh commented Nov 29, 2016

Looks like #734 includes #511 and #674, so I will wait for that before switching branches.

And yes – this is v1.13 only.

vieux pushed a commit to vieux/docker that referenced this pull request Nov 30, 2016
This makes the swagger.yaml useful outside of the documentation.
For background:
docker/docs#606 (comment)

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
(cherry picked from commit 54051b1)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
@mdlinville
Copy link

You'll now want to look at #756, though that's only there for audit purposes. If @thaJeztah approves it I will force-push vnext-engine to that state, which is the 21 patches in vnext-engine rebased on master. It's ugly, but trying to do a merge was too complicated.

@johndmulhausen
Copy link

@bfirsh Where are we on this? Do you plan to shut this PR down and make a new one pointed at vnext-engine? According to the Netlify staging, ReDoc isn't getting a swagger spec anymore

@bfirsh
Copy link
Contributor Author

bfirsh commented Dec 8, 2016

@johndmulhausen Yep! Just waiting for vnext-engine to be rebased and I'll sort it out.

I'm not really keeping a close eye on that, so feel free to ping me on here when I need to my bit.

@mdlinville
Copy link

@bfirsh this should be good to go after you rebase on current vnext-engine to resolve the conflict. I did the force-push this morning.

@mdlinville mdlinville changed the base branch from master to vnext-engine December 13, 2016 22:57
Makes more sense here instead of the Engine reference. It is
only consumed by the Engine.

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
See moby/moby#28319

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
Signed-off-by: Ben Firshman <ben@firshman.co.uk>
@johndmulhausen
Copy link

bitmoji

@mdlinville mdlinville merged commit 387dc47 into docker:vnext-engine Dec 14, 2016
@bfirsh
Copy link
Contributor Author

bfirsh commented Dec 14, 2016

Oh nice - thanks! Glad that just worked. 💃

@bfirsh bfirsh deleted the swagger-api-docs branch December 15, 2016 15:15
xianlubird pushed a commit to xianlubird/docker that referenced this pull request Dec 23, 2016
See: docker/docs#606

Also:
- Add missing redirects to API reference pages
- Remove v1.25 and 1.26, because they are being replaced with
  swagger generated docs.
- Remove all other docs which aren't reference material, because
  this can live in docker/docker.github.io

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
xianlubird pushed a commit to xianlubird/docker that referenced this pull request Dec 23, 2016
This makes the swagger.yaml useful outside of the documentation.
For background:
docker/docs#606 (comment)

Signed-off-by: Ben Firshman <ben@firshman.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants