feat:pre-commit Added pre-commit file and added to Python CI #112
Conversation
feefladder
force-pushed
the
pre-commit-ci
branch
from
3066d34 to
2d6c8c5
Compare
feefladder
force-pushed
the
pre-commit-ci
branch
from
2d6c8c5 to
6c9b997
Compare
| - repo: local | ||
| hooks: | ||
| - id: rust-fmt | ||
| name: cargo fmt | ||
| entry: cargo fmt --all -- --check | ||
| language: system | ||
| types: [rust] | ||
| stages: [pre-push] | ||
|
|
||
| - id: rust-clippy | ||
| name: cargo clippy | ||
| entry: cargo clippy --all --all-features --tests -- -D warnings | ||
| language: system | ||
| types: [rust] | ||
| stages: [pre-push] | ||
| pass_filenames: false | ||
|
|
||
| - id: rust-test | ||
| name: cargo test | ||
| entry: cargo test --all --all-features | ||
| language: system | ||
| types: [rust] | ||
| stages: [pre-push] | ||
| pass_filenames: false |
There was a problem hiding this comment.
Since these stages are pre-push, they don't run in CI as can be seen here
There was a problem hiding this comment.
I'm lukewarm about using pre-commit to manage Rust linting. pre-commit is mostly just used in the Python community IIRC
There was a problem hiding this comment.
hmm... there was a repo for rust-like linting, but it was rather stale, so I didn't use that.
I searched "Rust pre-commit" and found this commitizen blog where they use it also for rust stuff. Whether commitizen is wanted here I don't know (think not). also this rather large project using pre-commit almost the same as here. Also cargo-deny provides a pre-commit hook.
Then some rust-specific pre-commit info directly uses custom scripts (which I think is suboptimal)
I understand your lukewarm support though, it's a bit double to have it both in pre-commit and CI (and those going out of sync). Commitizen is python-based, but also supports rust.
However, I always spend wayy too much time having failing fmt/clippy tests in CI, so added it here, since I was working on pre-commit hooks anyways.
sidenote: these are crates replacing python pre-commit:
There was a problem hiding this comment.
I now added a comment to clarify that they do nothing unless explicitly installed through pre-commit install -t pre-push
I thought that way it's a clear opt-in for people like me?
| from ._async_tiff import * | ||
| from ._async_tiff import * # noqa: F403 | ||
| from ._async_tiff import ___version | ||
|
|
||
| if TYPE_CHECKING: | ||
| from . import store | ||
| from . import store # noqa: F401 |
There was a problem hiding this comment.
This gave ruff errors, not sure if we want to fix those or ignore like now
| Permission to use, copy, modify, distribute, and sell this software and | ||
| Permission to use, copy, modify, distribute, and sell this software and | ||
| its documentation for any purpose is hereby granted without fee, provided | ||
| that (i) the above copyright notices and this permission notice appear in | ||
| all copies of the software and related documentation, and (ii) the names of | ||
| Sam Leffler and Silicon Graphics may not be used in any advertising or | ||
| publicity relating to the software without the specific, prior written | ||
| permission of Sam Leffler and Silicon Graphics. | ||
|
|
||
| THE SOFTWARE IS PROVIDED "AS-IS" AND WITHOUT WARRANTY OF ANY KIND, | ||
| EXPRESS, IMPLIED OR OTHERWISE, INCLUDING WITHOUT LIMITATION, ANY | ||
| WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. | ||
| THE SOFTWARE IS PROVIDED "AS-IS" AND WITHOUT WARRANTY OF ANY KIND, | ||
| EXPRESS, IMPLIED OR OTHERWISE, INCLUDING WITHOUT LIMITATION, ANY | ||
| WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE. | ||
|
|
||
| IN NO EVENT SHALL SAM LEFFLER OR SILICON GRAPHICS BE LIABLE FOR | ||
| ANY SPECIAL, INCIDENTAL, INDIRECT OR CONSEQUENTIAL DAMAGES OF ANY KIND, | ||
| OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, | ||
| WHETHER OR NOT ADVISED OF THE POSSIBILITY OF DAMAGE, AND ON ANY THEORY OF | ||
| LIABILITY, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE | ||
| WHETHER OR NOT ADVISED OF THE POSSIBILITY OF DAMAGE, AND ON ANY THEORY OF | ||
| LIABILITY, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE |
closes #108
I also added rust pre-commit hooks that run on pre-push, since that prevents CI run failures I often encounter, while they should still not run in python CI (let's see when this PR does its CI)