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 opt-in support for the commit-msg hook #82

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

totph
Copy link
Contributor

@totph totph commented Feb 11, 2021

Enabled via the boolean config option revise.run-hooks.commit-msg

@totph
Copy link
Contributor Author

totph commented Feb 11, 2021

Does revise target a certain minimum git version? There are various ways to get the "hooks" path, newer versions have core.hooksPath, and for git worktrees these are not in $GIT_DIR but in the main .git.

@Manishearth
Copy link
Collaborator

I don't think we guarantee any specific minimum git version. I personally am OK with this optional feature requiring a very recent Git. I think --git-common-dir already handles worktrees though.

@totph totph force-pushed the msghook branch 2 times, most recently from 967e886 to a37f7c8 Compare May 4, 2021 13:20
@totph
Copy link
Contributor Author

totph commented May 4, 2021

Turns out the --git-common-dir argument was added in git v2.5 (and does not work in subdirs until 2.7.3), but git-revise fails with v2.4 anyhow, so no version check needed.

Also added core.hooksPath support.

@totph totph marked this pull request as ready for review May 4, 2021 13:31
Enabled via the boolean config option revise.run-hooks.commit-msg.
Works in git worktrees and respects core.hooksPath.
rwe added a commit to rwe/git-revise that referenced this pull request Sep 30, 2021
Instead of manually resolving from `self.workdir /`.

According to
mystor#82 (comment),
the current minimum supported git version is 2.5 or 2.7.3.

This means we can use --absolute-git-dir, which was introduced in 2.13.

--path-format=absolute was introduced in git 2.31.0, which at the time
of this writing is very recent, so a comment has been added to update
that remaining case when it's appropriate.
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