Re: [PATCH 0/3] pre-merge-hook

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 0/3] pre-merge-hook

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:38

Michael J Gruber [off-list ref] writes:
The pre-commit hook is often used to ensure certain properties of each
comitted tree like formatting or coding standards, validity (lint/make)
or code quality (make test). But merges introduce new commits unless
they are fast forwards, and therefore they can break these properties
because the pre-commit hook is not run by "git merge".

Introduce a pre-merge hook which works for (non ff, automatic) merges
like pre-commit does for commits. Typically this will just call the
pre-commit hook (like in the sample hook), but it does not need to.
When your merge asks for a help from you to resolve conflict, you
conclude with "git commit", and at that point, pre-commit hook will
have a chance to reject it, presumably.  That means for any project
that wants to audit a merge via hook, their pre-commit hook MUST be
prepared to look at and judge a merge.  Given that, is a separate
hook that "can just call the pre-commit but does not need to" really
needed and useful?

I admit that I haven't thought things through, but adding a boolean
"merge.usePreCommitHook" smells like a more appropriate approach to
me.

I dunno.

Re: [PATCH 0/3] pre-merge-hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

Junio C Hamano venit, vidit, dixit 06.09.2012 07:07:
Michael J Gruber [off-list ref] writes:
quoted
The pre-commit hook is often used to ensure certain properties of each
comitted tree like formatting or coding standards, validity (lint/make)
or code quality (make test). But merges introduce new commits unless
they are fast forwards, and therefore they can break these properties
because the pre-commit hook is not run by "git merge".

Introduce a pre-merge hook which works for (non ff, automatic) merges
like pre-commit does for commits. Typically this will just call the
pre-commit hook (like in the sample hook), but it does not need to.
When your merge asks for a help from you to resolve conflict, you
conclude with "git commit", and at that point, pre-commit hook will
have a chance to reject it, presumably.  That means for any project
that wants to audit a merge via hook, their pre-commit hook MUST be
prepared to look at and judge a merge.  Given that, is a separate
hook that "can just call the pre-commit but does not need to" really
needed and useful?

I admit that I haven't thought things through, but adding a boolean
"merge.usePreCommitHook" smells like a more appropriate approach to
me.

I dunno.
That would be an option ;)

Seriously, that would make the case where both hooks are the same even
simpler, obviously. On the other hand it would make the other case more
difficult. But, really, since non-automatic merges call pre-commit now,
and that is probably to stay, catching those with a different (part of
the pre-commit) hook will stay difficult anyways. So the question is
really more whether we prefer to bloat the config space or the name
space by this.

Either works for me, and if we don't change the current behaviour
(pre-commit-hook resp. no hook for non-automatic merges resp. automatic
merges) the config option is probably less confusing.

Michael
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help