chore: add pre-commit config and run linters #62
Conversation
.pre-commit-config.yaml
Outdated
There was a problem hiding this comment.
Would it be a good idea to move this to the .github folder to the rest of GitHub-specific configs?
There was a problem hiding this comment.
The .pre-commit-config.yaml is not GitHub specific, it's git specific, and should live at the project root
Adding pre-commit plugins to your project ¶
Add a file called
.pre-commit-config.yamlto the root of your project. The pre-commit config file describes what repositories and hooks are installed.
Once you've put it there it can be run from anywhere with pre-commit run --all-files or [the typical use] you can run pre-commit install which "installs" the hooks into the .git/hooks directory of your cloned git repo directory so that they run automatically whenever you git commit [this step is not strictly necessary if you let the CI bot run them and make "autofix commits", which we do here].
The autofix functionality is documented on the pre-commit.ci website:
auto fixing pull requests: if tools make changes to files during a pull request, pre-commit.ci will automatically fix the pull request.
| limitations under the License. |
There was a problem hiding this comment.
Is it possible to exclude this file from linting? I'm afraid some of the GitHub infra may consider this a custom license even if we add as much as a newline.
There was a problem hiding this comment.
- It is possible to exclude from linting using an exclude pattern (see docs)
- You needn't be afraid of that, this edit was introduced by the "end of file fixer" which I also use in other projects (e.g. fugit which do not invalidate upon linting.
Licenses aren't detected in a fragile way (like a MD5 checksum/exact match), we can tell that from e.g. MIT licensed projects where the license is detected despite the user editing it (you have to put your name in it for it to be valid).
Specifically the docs on how licenses are detected on GitHub using the open source Ruby gem licensee using several "matchers" which notes that its method includes
If we strip away whitespace and copyright notice,
Which confirms that the whitespace change is not going to obscure the license [it does in fact do more than that using string matching but as I say that wouldn't become relevant]
## [1.1.1](v1.1.0...v1.1.1) (2024-02-23) ### Docs * Performance observations for M2 CPUs (#56) ([8374ef6](8374ef6)), closes [#56](#56) ### Fix * Passing labels to `text_decoder` to compute loss. (#65) ([f445a8b](f445a8b)), closes [#65](#65) ### Improve * Larger batch benchmarks ([fdc8587](fdc8587)) ### Make * pre-commit config and linters (#62) ([0a3efac](0a3efac)), closes [#62](#62)
|
🎉 This PR is included in version 1.1.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
I've re-done the changes (by copying over the pre-commit exactly as in the previous PR #58 and executing it on this fresh branch). The only edits that were required to achieve a successful linting were the removal of
*imports (torchvision.transforms.*anduform.models.*specifically)Other than that nothing was changed, then the linters ran.
Some of these are the built-in ones from the pre-commit tool's own hooks:
Additionally:
pyproject.toml)pyproject.toml: pyflakes, pycodestyle, isort, pyupgrade)To run this on CI all you have to do is go to pre-commit.ci and give its GitHub Actions app permission to run on your CI (does not require a
.github/workflowsYAML config to run). The pre-commit config YAML file introduced in this PR is sufficient to configure the CI workflow to then run it.