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

Pass file names by default #281

Closed
wants to merge 4 commits into from
Closed

Pass file names by default #281

wants to merge 4 commits into from

Conversation

yarnabrina
Copy link

Description

Current hook definition stops passing file names as detected by pre-commit, and completely relies on the specifications in pyproject.toml.

Even though it's a good practice and quite common in python packages, I'd argue that end users (python developers using these packages) seldom configure with pyproject.toml. Their objective is just to run a tool to clean up the codebase as quickly as possible.

The changes in fork tries to resolve this by passing files from pre-commit by default (which is also the default for other standard linters - pylint, flake8, etc.).

If someone does configure vulture using pyproject.toml, they can still choose to use the current settings by defining paths in pyproject.toml and setting pass_filenames as false.

Related Issue

resolves #280

Checklist:

  • I have updated the documentation in the README.md file.
  • I have added an entry in CHANGELOG.md.
  • I have added or adapted tests to cover my changes.
  • I have run tox -e fix-style to format my code and checked the result with tox -e style.

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2022

Codecov Report

Merging #281 (d81ae23) into master (ebd643d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #281   +/-   ##
=======================================
  Coverage   99.37%   99.37%           
=======================================
  Files          18       18           
  Lines         635      635           
=======================================
  Hits          631      631           
  Misses          4        4           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebd643d...d81ae23. Read the comment docs.

@jendrikseipp
Copy link
Owner

I read up on this issue again and found the reason why things are configured the way they are: with pass_filenames: true pre-commit will only check modified files and therefore produce many false positives (see #244). While researching this, I found pre-commit run --all-files but I think the current solution of requiring the user to specify the files leads to better results.

I don't see a better solution than the status quo, so I'm closing this PR. But we can definitely reopen it when we find a better solution.

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

Successfully merging this pull request may close these issues.

passing filenames to pre-commit hook
3 participants