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

Store lefthook checksum in non-hook file #280

Conversation

mrexox
Copy link
Member

@mrexox mrexox commented Jun 21, 2022

Relates to #279

  • Store lefthook config checksum not in prepare-commit-msg, but in lefthook.checksum file.
  • Make sure config is synchronized when calling run command.

@mrexox mrexox force-pushed the feature/store-lefthook-checksum-in-another-file branch from fe2a57f to b759925 Compare June 21, 2022 15:29
@mrexox mrexox marked this pull request as ready for review June 21, 2022 15:32
@mrexox mrexox requested a review from skryukov June 21, 2022 15:32
@mrexox mrexox changed the title Store lefthook checksum in another file Store lefthook checksum in non-hook file Jun 21, 2022
@Envek
Copy link
Member

Envek commented Jun 21, 2022

Is it a good idea to still store checksums in the hooks directory? It is not a hook now.

Maybe we should store it in .git/info instead? 🤔

Also, as far as I remember, purpose of prepare-commit-msg hook was to detect config changes and install or uninstall hooks automatically. How it will be done now?

@mrexox
Copy link
Member Author

mrexox commented Jun 22, 2022

Is it a good idea to still store checksums in the hooks directory? It is not a hook now.

Maybe we should store it in .git/info instead?

Also, as far as I remember, purpose of prepare-commit-msg hook was to detect config changes and install or uninstall hooks automatically. How it will be done now?

@Envek Thank you! .git/info is much better directory for this purpose.

Well, while refactoring I have forgotten about this feature 😕. I have an idea how bring it back. We could store config file last updated timestamp with the checksum and compare it on every run command. It is cheaper that calculating checksum on every call.

If we find that the timestamp has changed, we can invoke install command, which compares the checksums and installs if necessary.

This might be not a very clean solution but I think it is more fair to let prepare-commit-msg hook to be just a usual configurable hook. (I missed this feature when I was using 0.* versions).

What do you think?

@mrexox mrexox force-pushed the feature/store-lefthook-checksum-in-another-file branch 2 times, most recently from a973c69 to 1a823d2 Compare June 24, 2022 04:29
@mrexox mrexox mentioned this pull request Jun 24, 2022
@mrexox mrexox added this to the 1.1.0 milestone Jun 27, 2022
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
@mrexox mrexox force-pushed the feature/store-lefthook-checksum-in-another-file branch from 9d46853 to 16be114 Compare July 25, 2022 14:20
mrexox added 2 commits July 25, 2022 17:30
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
@mrexox mrexox merged commit 42423cd into evilmartians:master Jul 25, 2022
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.

2 participants