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

insert-license hook ignores top-level files selected by types or types_or #52

Open
vlotorev opened this issue Jul 13, 2022 · 2 comments

Comments

@vlotorev
Copy link

vlotorev commented Jul 13, 2022

https://github.com/Lucas-C/pre-commit-hooks/blob/master/.pre-commit-hooks.yaml#L33 has line files: '.*/.*'. This overrides defaults and ignores top level files from project (top-level files don't have / in path when pre-commit is run).

Example when insert-license hook omits useful files (example uses language: fail just to print filenames passed by pre-commit):

repos:
  - repo: https://github.com/Lucas-C/pre-commit-hooks
    rev: v1.3.0
    hooks:
      - id: insert-license
        types: [makefile]
        language: fail
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.2.0
    hooks:
      - id: end-of-file-fixer
        types: [makefile]
        language: fail

The output for running pre-commit:

$ pre-commit run --all
Insert license in comments...............................................Failed
- hook id: insert-license
- exit code: 1

insert_license

doc/Makefile
src/Makefile

fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1

end-of-file-fixer

Makefile
doc/Makefile
src/Makefile

Note fix end of files hook from https://github.com/pre-commit/pre-commit-hooks prints top-level Makefile while insert-license doesn't.

@vlotorev
Copy link
Author

vlotorev commented Jul 13, 2022

Adding files: '' in user's .pre-commit-config.yaml restores the default behaviour and top-level files are passed correctly to entry command.
IMHO, it's better to remove all files: ... altogether from .pre-commit-hooks.yaml.

@Lucas-C
Copy link
Owner

Lucas-C commented Jul 14, 2022

I agree. Do you want to submit a PR?

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

2 participants