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

passing filenames to pre-commit hook #280

Closed
yarnabrina opened this issue May 25, 2022 · 2 comments
Closed

passing filenames to pre-commit hook #280

yarnabrina opened this issue May 25, 2022 · 2 comments

Comments

@yarnabrina
Copy link

I tried to use vulture as part of our .pre-commmit-config.yaml file. I added the following snippet directly as documented in README:

repos:
  - repo: https://github.com/jendrikseipp/vulture
    rev: 'v2.3'  # or any later Vulture version
    hooks:
      - id: vulture

It failed with the following error:

Please pass at least one file or directory

While I fixed it by passing . as an argument in args section or by using pass_filenames: true, I feel this is a general enough to be included as part of the hook definition. Can you please change this?

https://github.com/jendrikseipp/vulture/blob/master/.pre-commit-hooks.yaml#L7

Or, if there are any issues with that, is it possible to at least update the documentation with a working snippet? I think that will be very helpful for other users.

@jendrikseipp
Copy link
Owner

Thanks for bringing this to our attention? Would you care to make a pull request for one or both solutions you suggest?

@jendrikseipp
Copy link
Owner

Copying my comment from the PR:

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 a pull request may close this issue.

2 participants