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 Docs on Github pages #28

Merged
merged 15 commits into from
Jan 5, 2021
Merged

Conversation

spontanurlaub
Copy link
Collaborator

Closes #14.

With this pull request we can create a swagger documentation on github pages. Should be applied together with #25 , because all the methods from this pull request are documented here.

It also has a github action to update swagger UI from time to time and send a pull request automatically. I had to modify it, but I think it works (had no chance to try it out).

The documentation can be viewed at https://code1mountain.github.io/tdlight-telegram-bot-api/. It will later be available at https://tdlight-team.github.io/tdlight-telegram-bot-api/.

Activating github pages for the first time takes a few minutes, after that every change should be ready after about a minute (because of the github cdn).

@MarcoBuster MarcoBuster added the documentation Improvements or additions to documentation label Dec 16, 2020
@MarcoBuster
Copy link
Collaborator

Do you have an example of the auto-PR working?

@spontanurlaub
Copy link
Collaborator Author

I'm trying to trigger one manually, one moment...

@spontanurlaub
Copy link
Collaborator Author

image
I managed to get it to work by setting the version back manually. It sends a Pull request like this one

@MarcoBuster
Copy link
Collaborator

@code1mountain status? Waiting for review?

@spontanurlaub
Copy link
Collaborator Author

Yes, and also waiting for the other PR to be applied at the same time

@spontanurlaub
Copy link
Collaborator Author

Just saw that it got merged, we can merge this PR now I think. Is there still confusion if we want the documentation files in this repo or a separate one?

@MarcoBuster
Copy link
Collaborator

MarcoBuster commented Jan 4, 2021

Just saw that it got merged, we can merge this PR now I think. Is there still confusion if we want the documentation files in this repo or a separate one?

@cavallium what do you think? I vote for keep them in this repo

@@ -0,0 +1,13 @@
name: Validate API swagger documentation
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change this to "Documentation" and rename the file to "docs.yml". The name of a step should be as broad as possible, the information on what is currently doing is contained in the job.

@cavallium cavallium self-requested a review January 4, 2021 11:05
Copy link
Member

@cavallium cavallium left a comment

Choose a reason for hiding this comment

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

Remove "dist" directory from this repository, also add it in .gitignore.
Generated files shouldn't be on the repository.

@cavallium
Copy link
Member

@MarcoBuster I think that it's ok to put documentation in this repository, but only the source of the documentation.
Generated files and generated webpages shouldn't be on the repository.
If you want I can host the documentation, or we can use github pages. But not here on the master branch in a subfolder...

@MarcoBuster
Copy link
Collaborator

@cavallium Sure, that's fair.

@spontanurlaub
Copy link
Collaborator Author

We could also use GitHub actions and have all the generated/downloaded stuff in the gh-pages branch and host it with peaceiris/actions-gh-pages@v3
Then I would not check for an updated version on a schedule but just use the latest version on every commit. An other option is to have a separate repo with the generated/downloaded stuff and only the yaml file is in the repo.

The downside with this is that everyone who wants to edit the documentation and verify that it is still valid needs some external software, but https://editor.swagger.io also does that.

@spontanurlaub
Copy link
Collaborator Author

The nice thing with the GitHub pages action and the hosting in a separate branch is that every fork would have its own hosted documentation with all the changes made to it (if GitHub pages is enabled on the fork ofc).

The docs will now be hosted in a separate gh-pages branch and will update on every push to the main branch.
@spontanurlaub
Copy link
Collaborator Author

Please review @cavallium @MarcoBuster
All the html stuff is now in a separate branch called gh-pages that gets created with the latest version of swagger UI on every push to the main branch of this repository (not pull requests or anything else). Only the tdlight-api-openapi.yaml file and the icons/ folder are left in the main branch. The documentation is checked for validity on every push and pull request.

@cavallium
Copy link
Member

@code1mountain The branch is not "main" but it's "master"

Copy link
Member

@cavallium cavallium left a comment

Choose a reason for hiding this comment

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

Fix branch name

@spontanurlaub
Copy link
Collaborator Author

Okay, now it's working on the master branch

Copy link
Member

@cavallium cavallium left a comment

Choose a reason for hiding this comment

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

It's ok for me, I'll wait @MarcoBuster to merge

@luckydonald
Copy link
Collaborator

What I dislike about a separate branch is finding the correct docs for am older version would be incredible difficult.

Maybe we can make the automated commits at least contain the original commit hash so one has a chance to find it later?

@cavallium
Copy link
Member

@luckydonald Ok

@luckydonald
Copy link
Collaborator

Additionally, how would we handle pull requests with only a single branch? Would it automatically create a new gh-pages_brach123 for those?

How would the workflow for those branches look like?

Because my thinking is the following:
If you have to run the tool locally anyway to make sure everything is generating correctly before a merge of a PR (which should be checked, no?),
checking in those files in the same branch wouldn't really be any additional work.

@spontanurlaub
Copy link
Collaborator Author

What I dislike about a separate branch is finding the correct docs for am older version would be incredible difficult.

Maybe we can make the automated commits at least contain the original commit hash so one has a chance to find it later?

The separate branch mainly consists of a bunch of static html files and only the yaml file gets copied over there. If you want an older version of the docs, you can use the yaml file that sits in the master branch and view it with any swagger UI you want like https://editor.swagger.io. If we adapt to proper versioning in the future we will find a way to host the documentation for every release, but for now only the latest documentation is hosted. You can also copy the link to the raw openapi yaml file from a past version and enter it into the top bar of the documentation to view it.

@spontanurlaub
Copy link
Collaborator Author

Additionally, how would we handle pull requests with only a single branch? Would it automatically create a new gh-pages_brach123 for those?

How would the workflow for those branches look like?

Because my thinking is the following:
If you have to run the tool locally anyway to make sure everything is generating correctly before a merge of a PR (which should be checked, no?),
checking in those files in the same branch wouldn't really be any additional work.

The workflow for branches overwrites the same branch gh-pages for every push on master that changes the openapi yaml file. The documentation in new pull requests can be displayed like described in the last message, and it get's checked for syntax errors automatically.

@cavallium cavallium merged commit abcce9d into tdlight-team:master Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better documentation
4 participants