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

Remove pre-commit hook and build docs and examples in GH actions #141

Merged
merged 11 commits into from Mar 29, 2022

Conversation

ejm714
Copy link
Contributor

@ejm714 ejm714 commented Mar 22, 2022

Right now to make a contribution, a user needs to install the requirements and run make build which will re-create the example from the updated yml files. It seems preferable to let users just change the relevant .yml files (example table and/or checklist one) and then have the GH actions workflow run make docs and make examples and commit the changed filed. These will be the updated examples here: https://github.com/drivendataorg/deon/tree/main/examples plus the README which is rendered from a template that uses the checklist yml.

This PR also removes the pre-commit hook which is duplicative now that we use the GH actions workflow.

Example commit showing the files that get added: 8615756

Note: this action means that after a user opens their PR, they will need to run git pull or git push -f for any changes they make since the GH action will change the files on the remote branch.

@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #141 (b9293e4) into main (af69d34) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #141   +/-   ##
=======================================
  Coverage   96.80%   96.80%           
=======================================
  Files           6        6           
  Lines         188      188           
=======================================
  Hits          182      182           
  Misses          6        6           

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 af69d34...b9293e4. Read the comment docs.

@ejm714 ejm714 changed the title Remove pre-commit hook and run make build in GH actions Remove pre-commit hook and build docs and examples in GH actions Mar 22, 2022
@ejm714 ejm714 force-pushed the gh-actions-update branch 3 times, most recently from 8615756 to 16d8f9a Compare March 23, 2022 00:03
@ejm714
Copy link
Contributor Author

ejm714 commented Mar 23, 2022

Design considerations that I'd like feedback on:

  • this action means that after a user opens their PR, they will need to run git pull first or git push -f for any changes they make since the GH action will change the files on the remote branch.
  • because a commit is added after the one in which the tests were run, we lose the test summary on the PR description and the green checkmark on the PR list page given that commits made by the action don't trigger new workflow runs by default

@ejm714 ejm714 requested review from jayqi and pjbull March 23, 2022 00:16
@jayqi
Copy link
Member

jayqi commented Mar 23, 2022

An alternative approach is that we generate the make docs and make examples results in the PR workflow without committing, just for the preview, and we then generate them after merging into main and only actually commit them then.

@ejm714
Copy link
Contributor Author

ejm714 commented Mar 23, 2022

An alternative approach is that we generate the make docs and make examples results in the PR workflow without committing, just for the preview, and we then generate them after merging into main and only actually commit them then.

I like this approach and updated this PR accordingly. Ready for a review, the current tests are failing due to what appears to be a rate limiting issue with codecov. I'll re-run the tests a little later, a bunch of runs were done earlier today with the backlog after the downtime in GH actions

dev-requirements.txt Outdated Show resolved Hide resolved
@ejm714 ejm714 mentioned this pull request Mar 29, 2022
2 tasks
@ejm714
Copy link
Contributor Author

ejm714 commented Mar 29, 2022

@jayqi I've confirmed that the netlify pipeline builds the docs so changes get shown in the netlify preview on the PR (this is how it was already set). I've run a test to confirm changes to the yaml show up on the homepage and examples table.

The rendered checklists in varying formats which make examples creates are not relevant to the netlify preview since these link directly to github pages. Just testing that this command runs in the gh actions workflow (and not having it be part of the netlify piece) seems right.

Let me know if there's anything else needed on this PR.

Copy link
Member

@jayqi jayqi left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for getting all of this done.

@jayqi jayqi linked an issue Mar 29, 2022 that may be closed by this pull request
@ejm714 ejm714 merged commit ecff2fc into main Mar 29, 2022
@ejm714 ejm714 deleted the gh-actions-update branch March 29, 2022 22:56
@ejm714 ejm714 mentioned this pull request Mar 29, 2022
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.

Failed build on main branch: mkdocs and jinja2 incompatible versions
2 participants