search

jendrikseipp / vulture

Find dead Python code
MIT License
3.38k stars 148 forks source link

Pass file names by default #281

Closed yarnabrina closed 2 years ago

yarnabrina commented 2 years ago

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:

codecov-commenter commented 2 years ago

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 commented 2 years ago

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 https://github.com/jendrikseipp/vulture/pull/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.