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 "API Docs" GitHub Action to check if docs are up to date #324

Merged
merged 8 commits into from
Jun 15, 2021
Merged

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

merged 8 commits into from
Jun 15, 2021

Conversation

mahalrs
Copy link
Contributor

@mahalrs mahalrs commented Jun 14, 2021

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.

@mahalrs mahalrs changed the title Add API docs check Add "API Docs" GitHub Action to check if docs are up to date Jun 14, 2021
Copy link
Contributor

@nitishm nitishm left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

@nitishm
Copy link
Contributor

nitishm commented Jun 14, 2021

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

@mahalrs
Copy link
Contributor Author

mahalrs commented Jun 14, 2021

@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
Copy link
Contributor

nitishm commented Jun 14, 2021

@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
Copy link

codecov-commenter commented Jun 14, 2021

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 mahalrs requested a review from nitishm June 15, 2021 00:21
@mahalrs
Copy link
Contributor Author

mahalrs commented Jun 15, 2021

@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

Copy link
Contributor

@nitishm nitishm left a comment

Choose a reason for hiding this comment

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

Looks awesome!

@nitishm nitishm merged commit 9ada8f1 into Azure:main Jun 15, 2021
@mahalrs mahalrs deleted the docs-ci-bug branch June 16, 2021 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs generation failing in CI pipeline
3 participants