search

astral-sh / ruff-pre-commit

A pre-commit hook for Ruff.
Apache License 2.0
802 stars 38 forks source link

Consider not using --force-exclude by default #69

Open boenshao opened 4 months ago

boenshao commented 4 months ago

Contrary to #19, making --force-exclude an implicit default can cause confusing behaviors. As some CI environments might run under paths that are excluded by default or excluded by custom configuration.

For example, the Bitbucket pipeline runs under /opt/atlassian/pipelines/agent/build, which is excluded by default (build), so running pre-commit run --all-files takes no effect at all, but it totally works as expected under local repo, really confusing!

Take me quite some time to figure out why our CI is not working, apparently, others have the same problem.

I suggest making the --force-exclude an opt-in, or at least add a reminder in README for setting --no-force-exclude.

schlerp commented 4 months ago

Agreed, this has recently caught me out too, i think the best option is to just add it to the readme for now. Its trivial to remove if we do make the decision to drop it as a default argument. I have submitted a PR for this here: #72

charliermarsh commented 4 months ago

Alternatively, I'm going to remove build from the default exclusion list in v0.3.0. It causes all sorts of problems (like this), and it's the only default for which we get issues like this.

charliermarsh commented 4 months ago

See: https://github.com/astral-sh/ruff/pull/10093.

schlerp commented 4 months ago

Yeah that makes sense :) Do we still think it's worth mentioning its the default? I spent quite a bit of time digging into this and really would have appreciated it being pointed out in the README 😆