Add make command for previewing API docs#29078
Conversation
bfirsh
changed the title
bfirsh
force-pushed
the
add-swagger-docs
branch
from
9d9faf7 to
e5e3f09
Compare
Makefile
Outdated
There was a problem hiding this comment.
Not sure this is great.
Maybe we can move this into a hack/make/swagger-docs script that can just do a -p and then report the port being exposed.
Or an env var like DOCKER_SWAGGER_PORT.
But still set the host port to something like 3000 instead of 80.
cpuguy83
left a comment
cpuguy83
left a comment
There was a problem hiding this comment.
One issue, otherwise this is really awesome.
bfirsh
force-pushed
the
add-swagger-docs
branch
from
e5e3f09 to
de7e7c0
Compare
|
@cpuguy83 Fixed! |
Makefile
Outdated
There was a problem hiding this comment.
can you add a ## comment here? We use it for make help to show a nice usage description ❤️
|
Thanks @bfirsh this should get reviewers going do you think we can add some basic skinning from the docs.docker.com site, or would that be difficult?
|
|
@thaJeztah It doesn't have the docs.docker.com skin - that's how it looks. The only thing broken is the colour missing in the top-left. I can add that when I get time, but I don't think it's critical. If this gets merged before I'll get to it I'll open a follow-up to fix it. :) |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
I'm okay with fixing the nits in a follow up
LGTM
Makefile
Outdated
There was a problem hiding this comment.
I still think this should be configurable.
bfirsh
force-pushed
the
add-swagger-docs
branch
from
de7e7c0 to
7a290c5
Compare
bfirsh
force-pushed
the
add-swagger-docs
branch
2 times, most recently
from
64c17a0 to
273ddcd
Compare
Makefile
Outdated
There was a problem hiding this comment.
oh! looks like it has to be on the line below, make help doesn't seem to pick it up if it's here 😢
we can fix it in a follow up if needed, no blocker
There was a problem hiding this comment.
ooops my bad. fixed!
Much easier than the previous method of copying over to the docs repository and generating the docs. And, as @cpuguy83 pointed out, that actually didn't work because the PR that adds Swagger docs isn't merged yet. Oopsy. Signed-off-by: Ben Firshman <[email protected]>
bfirsh
force-pushed
the
add-swagger-docs
branch
from
273ddcd to
27ca33e
Compare
|
Merging since tests won't do anything for this PR. |
5 participants
Much easier than the previous method of copying over to the docs
repository and generating the docs.
And, as @cpuguy83 pointed out, that actually didn't work
because the PR that adds Swagger docs isn't merged yet. Oopsy.
Signed-off-by: Ben Firshman [email protected]