-
Notifications
You must be signed in to change notification settings - Fork 122
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
pre-commit autoupdate seems to fail #571
Comments
Thanks for the report I see that pre-commit support was added here #499 I am not very familiar with pre-commit internals. Can you please provide a self contained example so that I can reproduce this error? |
I think the minimal example is to use what it proposed in the docs, so a
|
It is worth noting that this happened from one day to another; it is not the first time I use towncrier with pre-commit |
I was not able to reproduce see the change from #572 and the result https://results.pre-commit.ci/run/github/48647797/1703077437.QjeBDGruRIWq9RS5mZgEHg |
I see that in there you test only for the direct installation, you are not using I can install the git hook, what fails is the update |
Thanks for the info. I think I got this. I was able to reproduce the error and I have pushed the steps to the PR. Not sure what is going in there. I will not have much time to investigate this. My main concern was why our automated tests pass, while people are getting errors. I am happy to review a fix for this. Regards |
I was able to reproduce the error here https://github.com/twisted/towncrier/actions/runs/7276149695/job/19825390531?pr=572#step:6:18 |
Thanks for the report/PR already - also suffering: [https://github.com/twisted/towncrier]
=====> /tmp/tmp1orupgt7/.pre-commit-hooks.yaml is not a file Only started since adding the |
We alrady have this issue #571 for looking into this error on If you have time, please consider opening an issue in pre-commit repo and linking to this issue or PR Link to reproducing the error https://github.com/twisted/towncrier/actions/runs/7276149695/job/19825355701?pr=572#step:6:1 |
Only the I've updated the Now this results in:
And as long as in the future we actually tag the released branch rather than the pre-rebased one, it'll work fine. |
I think that is best to reopen this and fix it only after we have updated the release documentation. Right now we merge PR with squash. The release PR should be merged witout squash I am trying to fix this in #571 @SmileyChris if you have time, please take a look at that PR and see if the release process makes sense. Thanks! |
Another thing is that it's good to have |
Agreed that it would be good to have them as part of the automated tests. The release PR can be merged with squash if the tag is applied to the squashed commit rather than on the branch. But yeah, probably fine just better to not squash in the first place :) |
We use GitHub Releases to do the release... so the tag is created by GitHub... on a commit from the release branch. I think that we should just not use squash in the first place But I don't think that we can enforce this using |
…t autoupdate. (#572) * Use pre-commit dog food. * Run pre-commit autoupdate. * Update .github/workflows/ci.yml * Better name. Also run auto-update. * Add news fragment. * Update black. * Update pre-commit hooks. * Use latets towncrier. * Use latest pre-commit-hooks. * Use immutable versions * Add info about release process. * Apply fix from Sadik. * Update .github/workflows/ci.yml Co-authored-by: Chris Beaven <[email protected]> --------- Co-authored-by: Hynek Schlawack <[email protected]> Co-authored-by: Chris Beaven <[email protected]>
When I launch pre-commit autoupdate I get this
So all other git hooks get updated, but it crashes only when trying to update this project.
I also tried to manually update the config file with what I see from your docs,
but the result doesn't change.
Also
pre-commit clean
doesn't seem to help.The text was updated successfully, but these errors were encountered: