drivendataorg / deon

A command line tool to easily add an ethics checklist to your data science projects.
https://deon.drivendata.org/
MIT License
289 stars 52 forks source link

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

Closed ejm714 closed 2 years ago

ejm714 commented 2 years ago

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: https://github.com/drivendataorg/deon/pull/141/commits/8615756e4fb2ce879a083f262ff04968ad5a0241

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[bot] commented 2 years ago

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 commented 2 years ago

Design considerations that I'd like feedback on:

jayqi commented 2 years ago

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 commented 2 years ago

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

ejm714 commented 2 years ago

@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.