You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I need to write a hook to ensure empty files are not being committed, looking around the issues section it seems this has not been mentioned yet. Is this something considered generic/useful enough to be included in this repo? If so we could discuss further details about it and I can end up making a PR for it.
this would break on most python projects since empty __init__.py is a best practice
Yes indeed, I was planning on adding a --whitelist argument with filenames that can be empty, with __init__.py being included in the defaults. What do you think?
--allowlist is the acceptable name for that. but that's not a great solution either since the files would still get passed to the hook wasting execution time (exclude is how you're supposed to exclude files in pre-commit)
since it would needexclude by default that makes it not that useful out of the box I imagine
--allowlist is the acceptable name for that. but that's not a great solution either since the files would still get passed to the hook wasting execution time (exclude is how you're supposed to exclude files in pre-commit)
since it would needexclude by default that makes it not that useful out of the box I imagine
Makes sense, thanks for the insights, I have taken them into account and created #836.
It works out of the box, end users do not need to add any config since the default exclude is set directly in .pre-commit-hooks.yaml.
I personally tested it with the following .pre-commit-config.yaml file:
yeah I still don't think this is good enough by default -- there's other valid reasons to have empty files that we can't account for that don't have a convention (for instance keeping a directory around)
Fair enough. I guess there are lots of projects which have no valid reasons (like the ones I am working on haha) for committing empty files and lots that do. I see this as just falling under the normal spectrum of hook use. I improved the documentation on the hook in order to cover excluding other false positives, that may help users who wish to adopt this hook (if added).
Other than that if this is still not a good fit for the repo I guess there is not much to do besides thanking you for taking the time to review the issue. Have a good one :)
Activity
asottile commentedon Nov 10, 2022
this would break on most python projects since empty
__init__.pyis a best practiceantonag32 commentedon Nov 10, 2022
Yes indeed, I was planning on adding a
--whitelistargument with filenames that can be empty, with__init__.pybeing included in the defaults. What do you think?asottile commentedon Nov 10, 2022
--allowlistis the acceptable name for that. but that's not a great solution either since the files would still get passed to the hook wasting execution time (excludeis how you're supposed to exclude files inpre-commit)since it would need
excludeby default that makes it not that useful out of the box I imagineantonag32 commentedon Nov 10, 2022
Makes sense, thanks for the insights, I have taken them into account and created #836.
It works out of the box, end users do not need to add any config since the default
excludeis set directly in.pre-commit-hooks.yaml.I personally tested it with the following
.pre-commit-config.yamlfile:asottile commentedon Nov 10, 2022
yeah I still don't think this is good enough by default -- there's other valid reasons to have empty files that we can't account for that don't have a convention (for instance keeping a directory around)
antonag32 commentedon Nov 11, 2022
Fair enough. I guess there are lots of projects which have no valid reasons (like the ones I am working on haha) for committing empty files and lots that do. I see this as just falling under the normal spectrum of hook use. I improved the documentation on the hook in order to cover excluding other false positives, that may help users who wish to adopt this hook (if added).
Other than that if this is still not a good fit for the repo I guess there is not much to do besides thanking you for taking the time to review the issue. Have a good one :)