Include GitHub Actions #98
Conversation
|
List of checks that failed: trim trailing whitespace, fix end of files, fix double quoted strings, black-jupyter, reorder python imports, add trailing commas, pyupgrade, autopep8, flake8 List of checks that passed: check yaml, debug statements (python), check docstring is first, check json We may want to review which checks we actually want to run and make an additional issue to resolve the failing checks? |
|
Ok pre-commits are now working properly and conflicts between black-jupyter and autopep8, as well as the double vs single quotes, are resolved (thanks to some suggestions from Anissa on the former!) Anyways, this should be ready for review now. |
There was a problem hiding this comment.
I'm not sure of the best place for it (maybe the README for now, but eventually in a "developer's guide" type page?), but we should include instructions on how to enable pre-commit locally. I think we want to add it to cupid-dev, and then recommend using that environment when committing changes. The real power of the package is that it will warn you when you try to commit changes that violate the style guide; using it in Github Actions is more of a backup plan than anything else.
Other than that (and my confusion around annotate), this all looks great! Little is as satisfying as seeing a whole bunch of trailing whitespace get removed :)
| # If extensions (or modules to document with autodoc) are in another directory, | ||
| # add these directories to sys.path here. If the directory is relative to the | ||
| # documentation root, use os.path.abspath to make it absolute, like shown here. | ||
| import os | ||
| import sys | ||
| from __future__ import annotations |
There was a problem hiding this comment.
It doesn't look like annotations is used anywhere; was this requested by something in the CI testing?
| @@ -1,92 +1,100 @@ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
same comment; I assume it's getting added by the CI but I don't understand why
|
I added a note on running I think the |
There was a problem hiding this comment.
I added a note on running
pre-commit runto our contributor's guide. If you prefer this goes in the readme, I could add it there too, but it seems to fit better in the wiki page (although perhaps it is a little hidden).
The contributor guide is a great place for it! I didn't think to look there, and the wiki files don't show up as changed in the PR :) I'll come back this afternoon with a small update (adding precommit to environments/dev-environment.yml and giving more explicit instructions in the wiki about setting it up to run with every git commit) but I'll merge this now
|
Sounds great! Thanks for reviewing and merging this, Mike! |
Closes #97
This PR uses some of the pre-commit-hooks from this repository and an updated workflow from here.