Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
boenshao opened this issue Feb 20, 2024 · 6 comments
Open

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

boenshao opened this issue Feb 20, 2024 · 6 comments

Comments

@boenshao
Copy link

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 added a commit to schlerp/ruff-pre-commit that referenced this issue Feb 23, 2024
Updates README.md to include a warning about the default choice of `--force-exclude` being used by the github action. This warning includes links to the documentation for these settings. Addresses astral-sh#69
@schlerp
Copy link

schlerp commented Feb 23, 2024

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
Copy link
Member

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
Copy link
Member

See: astral-sh/ruff#10093.

@schlerp
Copy link

schlerp commented Feb 23, 2024

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 😆

charliermarsh added a commit to astral-sh/ruff that referenced this issue Feb 28, 2024
## Summary

This is a not-unpopular directory name, and it's led to tons of issues
and user confusion (most recently:
astral-sh/ruff-pre-commit#69). I've wanted to
remove it for a long time, but we need to do so as part of a minor
release.
nkxxll pushed a commit to nkxxll/ruff that referenced this issue Mar 10, 2024
## Summary

This is a not-unpopular directory name, and it's led to tons of issues
and user confusion (most recently:
astral-sh/ruff-pre-commit#69). I've wanted to
remove it for a long time, but we need to do so as part of a minor
release.
@ryanpeach
Copy link

ryanpeach commented Sep 10, 2024

I don't want to run with --force-exclude. This should be an option. Also I went crazy for an hour trying to figure out why it was ignoring files I had in a .github/scripts folder (hidden, thus excluded). Please move this to args and document it.

@ryanpeach
Copy link

Actually, sorry. I'm still going crazy. --force-exclude doesn't seem to change whether or not my .github/scripts folder is evaluated. It works in cli but not in pre-commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants