Azure / orkestra

Orkestra is a cloud-native release orchestration and lifecycle management (LCM) platform for the fine-grained orchestration of inter-dependent helm charts and their dependencies
https://azure.github.io/orkestra
Other
105 stars 16 forks source link

Add "API Docs" GitHub Action to check if docs are up to date #324

Closed mahalrs closed 3 years ago

mahalrs commented 3 years ago

Closes: #311

This change add a GitHub Action to check if API docs are up to date. If not, this check will fail and you should run make api-docs before pushing your changes (similar to golangci lint check).

NOTE 1:

Since the main branch is protected, Github action cannot use regular GITHUB_TOKEN, it needs special privileges. But if we create a Personal Access Token with write access, any collaborator can make changes to the workflow and can misuse the token.

https://github.community/t/how-to-push-to-protected-branches-in-a-github-action/16101/5 The token will not be able to push to a protected branch as that would enable anyone with write access to your repo to push to that protected branch by simply updating the workflow in a branch. Having that ability would make protected branches useless.

So we should create a required check (similar to e2e, golangci-lint) to see if api docs are up to date. If not, this check will fail and the person making the PR (or pushing directly to main) should run make api-docs to update the docs before pushing/merging changes.

@nitishm @jonathan-innis Let me know if you have any suggestions such as if there is a way collaborators can't misuse the token by making changes to workflow action.

NOTE 2:

This check will fail because I have added a line to ./docs/api.md that makes docs outdated. So, I must run make api-docs and push new changes to pass all checks.

EDIT: Updated api docs by running make api-docs locally and committed new changes. Now API Docs check should pass.

nitishm commented 3 years ago

@mahalrs We should add this step to contributors/developers docs as well

mahalrs commented 3 years ago

@mahalrs We should add this step to contributors/developers docs as well

You mean checking if contributors/developers docs are up to date OR adding a note to contributors/developers docs that they must run make api-docs before pushing changes.

nitishm commented 3 years ago

@mahalrs We should add this step to contributors/developers docs as well

You mean checking if contributors/developers docs are up to date OR adding a note to contributors/developers docs that they must run make api-docs before pushing changes.

The latter

codecov-commenter commented 3 years ago

Codecov Report

Merging #324 (daa8b1e) into main (47d8dc3) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #324   +/-   ##
=======================================
  Coverage   23.24%   23.24%           
=======================================
  Files          12       12           
  Lines         783      783           
=======================================
  Hits          182      182           
  Misses        594      594           
  Partials        7        7           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 47d8dc3...daa8b1e. Read the comment docs.

mahalrs commented 3 years ago

@mahalrs We should add this step to contributors/developers docs as well

You mean checking if contributors/developers docs are up to date OR adding a note to contributors/developers docs that they must run make api-docs before pushing changes.

The latter

Done