-
-
Notifications
You must be signed in to change notification settings - Fork 838
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
Introduce .pre-commit-hooks.yaml as a replacement for hooks.yaml #470
Conversation
d1c902a
to
b9e5184
Compare
I like the move and new name. It does seem a little verbose but it's not too bad. It's an easy fix for hook authors. My main concern would be: what happens when someone runs
I'm not sure how to avoid that. Hook authors could duplicate (maybe symlink works?) |
Yeah I thought about that too -- I think during a transition period hook authors would exactly duplicate the two files (as is done in this PR). I considered a symlink but it has worse portability (notably for windows). After the transition period, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems reasonable to me. transitions suck but I think this is a definitely positive change.
I've gone ahead and updated all of our repositories (and all of my repositories) for this change. I plan to create issues on projects on http://pre-commit.com/hooks.html something along the lines of:
|
In pre-commit/pre-commit#470, hooks.yaml was renamed to .pre-commit-hooks.yaml to try to make the file more likely for upstreams to include. New versions of pre-commit will support both, but issue a warning about the hooks.yaml name. Moving it and setting up a symlink for compatibility should work on all versions.
/o\ I'm so sorry for all the email spam, please don't hate me :) |
Question: I currently have both |
@Lucas-C the new file is .pre-commit-hooks.yaml, different from the config file |
Oh sorry, silly me :) |
pre-commit/pre-commit#470 moved the default location of the hook config file bump version to 0.1.4 closes #1
I think this will make upstreams more likely to accept a PR which adds this metadata if we have a namespaced files.
(at the cost of an annoying migration -- sorry I chose a bad name in the project's infancy)
Is this a good idea?
Here's the output this generates when committing in this project:
maybe a little too verbose? suggestions for something shorter?