Skip to content

Proposal: Check empty files #834

Closed
Closed

Description

@antonag32

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.

Activity

  1. asottile commented on Nov 10, 2022

    @asottile
    Member

    this would break on most python projects since empty __init__.py is a best practice

  2. antonag32 commented on Nov 10, 2022

    @antonag32
    Author

    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?

  3. asottile commented on Nov 10, 2022

    @asottile
    Member

    --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 need exclude by default that makes it not that useful out of the box I imagine

  4. antonag32 commented on Nov 10, 2022

    @antonag32
    Author

    --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 need exclude 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:

    repos:
    -   repo: https://github.com/antonag32/pre-commit-hooks
        rev: 94ed7581c7af89733bacb827afcb84f020257a5b
        hooks:
        -   id: forbid-empty-files
    
  5. asottile commented on Nov 10, 2022

    @asottile
    Member

    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)

  6. antonag32 commented on Nov 11, 2022

    @antonag32
    Author

    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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions