Improve pre commit hook #17595
Improve pre commit hook #17595
Conversation
The documentation state that the function should return, but actually it exists. This is repaired. Removing `set -e` ensure the script does not stop if the linter fails. This is probably what we expected given that we put the return value in a variable. This would ensure that the patch that was saved is always restored. I don't expect this to cause issues because the remaining of the changes are not destructive. I manually tested that the bug can't be reproduced with this script. However, I've not tested the change in case of `git apply` failure, as I don't know how to force apply to fail. Fixed: ankidroid#17593
The variable meaning was not clear. It seemed to indicate it was actually the diff.
This ensures that, in case we have an issue and restore does not work, a future `git commit` don't overwrite the patch file.
Arthur-Milchior
force-pushed
the
pre-commit
branch
from
e305041 to
0b09a82
Compare
|
How does this code differ from the currently generated hook? Can a subset of this be submitted upstream? |
|
The base of the bug is that we had Patch 2 by itself there is no reason to just ask them to rename a variable. Patch 3 may be nice. But honestly, I added just for extra safety. When their code work as expected (i.e. we have not added |
david-allison
added
the
Needs Second Approval
mikehardy
added
Pending Merge
github-actions
bot
removed
the
Pending Merge
I expect first commit to be sufficient. Commit three is probably overkill. But I don't want to lose hours of work anymore. And the slight risk that, if we stop overwritting the same file all the time, we save data seems quite nicer than risking a repeat.
Fixed: #17591