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

OpenAPI documentation #37

Merged
merged 17 commits into from
Nov 1, 2021
Merged

OpenAPI documentation #37

merged 17 commits into from
Nov 1, 2021

Conversation

CubicrootXYZ
Copy link
Collaborator

Add OpenAPI documentation compatible with swag and a workflow to push a static html to website

@CubicrootXYZ CubicrootXYZ self-assigned this Sep 30, 2021
@CubicrootXYZ CubicrootXYZ linked an issue Sep 30, 2021 that may be closed by this pull request
@eikendev eikendev mentioned this pull request Sep 30, 2021
@eikendev
Copy link
Member

I see three valid approaches here:

  1. The logic to generate the doc lives in /server. That's the case with your proposed workflow. We will need to push a commit to another repository, which I don't know I like too much.
  2. The logic to generate the doc lives in /website. A push to /server triggers a workflow that somehow triggers a workflow in /website (need to check if this is easily doable). That workflow now clones /server, builds the doc, and then the website.
  3. The logic to generate the doc lives in /website. Our /website repository has /server as a submodule (which pins to a specific commit), which is checked out when /website is built. When we think it is time to update our docs, we update the commit of our submodule in /website.

I feel like (3) is the cleanest, but we will not have a doc for the latest commit in /server without manually keeping up to date. Anyway, if we only show the doc of one specific version, that version should be a "stable" one. Now I know PushBits is not as far as having a "stable" version, but maybe for the time being we agree on the submodule commit individually? Which option do you prefer?

@CubicrootXYZ
Copy link
Collaborator Author

I am no fan of manual interaction as it is pron to errors. If you do not like the logic for generatin the docu in this repo I'd go with 2. we can use repository dispatches for that.

@eikendev
Copy link
Member

eikendev commented Oct 3, 2021

My main concern is using a personal access token (PAT) that has a broader scope than needed. I do not think there is currently a way to restrict a PAT to a subset of repositories. If we can work around that, I'm fine with (1) and (2) also. It seems like deploy keys can also be used to write to a repository. Maybe we could put a deploy key for /website as a secret in /server, and use it to push the generated artifacts?

@CubicrootXYZ
Copy link
Collaborator Author

Ah sorry overlooked that the dispatches needs auth too. Yeah I think the deploy key should work best then. Let's try with one.

@eikendev
Copy link
Member

We now have a WEBSITE_DEPLOY_KEY in this repo's environment. It's a private SSH key, and I think to use it we need to store it somewhere as a file during deployment. It has write access to /server.

@CubicrootXYZ
Copy link
Collaborator Author

CubicrootXYZ commented Oct 11, 2021

Implemented the push to the website repo - works well.

However I do not know where the warnings about multiple declarations of routes come. They are defined only once.

We know need to think about:

  1. A contact url (or leave empty)
  2. Inprint/legal stuff

@CubicrootXYZ CubicrootXYZ added the documentation Improvements or additions to documentation label Oct 11, 2021
@eikendev
Copy link
Member

Nice, I wasn't aware this already pushes into /website. I've enabled GitHub Pages for it, so we can now see it live at https://pushbits.github.io/website/. For the contact information, we can link to my (and if you like your) website. For legal stuff, we should give the license of the source (ISC). Anything else on your mind?

@CubicrootXYZ
Copy link
Collaborator Author

I added your website as contact, linked the pushbits team as general URL and added the license. For me it is ready to merge.

Copy link
Member

@eikendev eikendev left a comment

Choose a reason for hiding this comment

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

Looks good, just wondering about the version. Also, I've changed the path where the new file will be dropped.

cmd/pushbits/main.go Show resolved Hide resolved
@eikendev eikendev merged commit c723287 into master Nov 1, 2021
@eikendev eikendev deleted the api-doc branch November 1, 2021 16:52
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 API documentation
2 participants