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

feat: Build framework for generating API docs #5383

Merged
merged 99 commits into from
Dec 19, 2022

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Dec 12, 2022

Related: #3522
Preview: link

Changes:

  • Expose the swagger doc.json via chi.Router
  • Add feature flag for enabling swagger endpoint (disabled by default)
  • Write multiple .md docs pages, grouped by tags like Workspaces, Templates, etc.
  • Update manifest.json file to include API doc pages

Doc pages are generated from swagger.json using widdershins and a custom, modified template. The widdershins generates a single Markdown file, so scripts/apidocgen/postprocess cuts it into .md files, and updates the manifest.json file.

schemas.md contains all request/response entities linked from other .md files.

Follow-up:

  • Add missing // swagger comments to .go files in coderd as this PR has already 30+ files modified
  • Write validators to make sure that // swagger comments are inline with chi router

@mtojek mtojek self-assigned this Dec 12, 2022
@mtojek mtojek requested a review from bpmct December 16, 2022 16:13
@mtojek
Copy link
Member Author

mtojek commented Dec 16, 2022

Thanks for all comments, @mafredri! I guess that I cleared the majority of them. Looking forward to a review from Ben, and if successful, we can get this change in.

Comment on lines 410 to 411
# Mark all generated files as fresh so make thinks they're up-to-date. This is
# used during releases so we don't run generation scripts.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanity check: New API docs (swagger and markdown) are being generated (and committed) during each release? Is this also how we generate our Prometheus docs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both actions, Prometheus and API docs, use the same target, make gen, so every generated doc is stored in the repository. If you see possible drawbacks, we can sync offline.

Copy link
Member

@bpmct bpmct left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through the docs pages and I'm a huge fan of the format! Having the API docs available on our public docs will make it easy to consume & search. And providing sample CURLs is 👌🏼

I'm almost ready to approve but had a few questions related to your follow-up notes:

Add missing // swagger comments to .go files in coderd as this PR has already 30+ files modified

I'm assuming this follow-up PR will add documentation for all of the available routes since the current docs are incomplete. Do you have a sense of how long this will take, and whether we should add a notice in the meantime saying something similar to "this page is incomplete, stay tuned." I don't want it to seem like we only have ~6 API routes

Write validators to make sure that // swagger comments are inline with chi router

Does this mean that, essentially, every API route has to be documented? If so, it will be awesome to share that we have API docs for essentially every use case our dashboard or CLI handles. Of course, with some minor nuances (e.g. create template version, lookup org id, etc).

@mtojek
Copy link
Member Author

mtojek commented Dec 19, 2022

Hey @bpmct!

I'm assuming this follow-up PR will add documentation for all of the available routes since the current docs are incomplete. Do you have a sense of how long this will take

It may take a while as we need to cover all routes and model structures, and if you look at coderd and codersdk package we have plenty of them :) I would book 2-3 days to cover those, and address potential suggestions during code reviews.
In the meantime, I'd like to prepare some validation/test to verify that our docs are as much aligned with code as possible.

and whether we should add a notice in the meantime saying something similar to "this page is incomplete, stay tuned." I don't want it to seem like we only have ~6 API routes

Definitely a good idea, I will work on it now.

Does this mean that, essentially, every API route has to be documented? If so, it will be awesome to share that we have API docs for essentially every use case our dashboard or CLI handles. Of course, with some minor nuances (e.g. create template version, lookup org id, etc).

Correct, that's the assumption :)

@mtojek mtojek merged commit dc6d271 into coder:main Dec 19, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants