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

Add pre-commit hooks configuration #7

Merged

Conversation

skycaptain
Copy link
Contributor

This adds support for using this repo as pre-commit hook (https://pre-commit.com/). To add editorconfig-checker to your repository add this to your .pre-commit-config.yaml:

repos:
-   repo: https://github.com/editorconfig-checker/editorconfig-checker.python
    rev: ''  # pick a git hash / tag to point to
    hooks:
    -   id: editorconfig-checker

pre-commit also supports building go packages directly from repository, however this would require go on the client. Since pre-commit is also written in Python, and thus no further dependency is produced, this wrapper is preferred instead. See editorconfig-checker/editorconfig-checker#133 for details.

The use of types: [text] is debatable. If I understood the documentation correctly, it filters files which are recognized by Git as text files before passing to editorconfig-checker. This is merely an optimization and can be overwritten by the user if necessary. However, if Git recognizes a file with the wrong type at might be silently ignored. @mstruebing opinions? Is this even necessary or would our just pass all files to editorconfig-checker?

This adds support for using this repo as pre-commit hook (https://pre-commit.com/). To add editorconfig-checker to your repository add this to your .pre-commit-config.yaml:

repos:
-   repo: https://github.com/editorconfig-checker/editorconfig-checker.python
    rev: 2.1.0
    hooks:
    -   id: editorconfig-checker

pre-commit also supports building go packages directly from repository, however this would require go on the client. Since pre-commit is also written in Python, and thus no further dependency is produced, this wrapper is preferred instead. See editorconfig-checker/editorconfig-checker#133 for details.
@mstruebing
Copy link
Member

Is this even necessary or would our just pass all files to editorconfig-checker?

editorconfig-checker tries to filter out non-text files too, so it would be filtered anyway if you would pass it to editorconfig-checker.
I might think that pre-commit does a slightly better job in filtering so I would let pre-commit do the filtering.

@mstruebing
Copy link
Member

@mmicu any opinions or blocker about this?

@mstruebing
Copy link
Member

ping @mmicu

@mmicu
Copy link
Member

mmicu commented Nov 21, 2020

Hi. I think that it could be an useful tool to use. I'm going to review and merge the commit. Thanks for your pull request.

@mmicu mmicu merged commit 483258f into editorconfig-checker:master Nov 21, 2020
@skycaptain skycaptain deleted the feature/add-pre-commit-hook branch November 21, 2020 22:35
@skycaptain
Copy link
Contributor Author

Thanks @mmicu. Would it make sense to also commit the README.md change to the parent project's README?

@skycaptain
Copy link
Contributor Author

You would also need to release (or atleast tag) a new version to make this change work.

@mstruebing
Copy link
Member

I think it would make sense, also it would make sene to mention here: https://github.com/editorconfig-checker/editorconfig-checker.github.io

Feel free to add documentation :)

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 this pull request may close these issues.

None yet

3 participants