* pre-commit hook not run on conflict resolution during rebase
@ 2015-05-28 19:38 Stefan Haller
2015-05-28 20:03 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Haller @ 2015-05-28 19:38 UTC (permalink / raw)
To: git
When a rebase stops because of a conflict, and I edit the file to
resolve the conflict and say "git rebase --continue", then the
pre-commit hook doesn't run at that point, which means that I can commit
bad stuff which the pre-commit hook would normally not allow in. We were
bitten by this a few times already. (In our case, the hook rejects code
that hasn't been run through clang-format. That's easy to forget when
resolving conflicts during a rebase.)
>From glancing through the githooks manpage, I couldn't see any other
hook that would help in this situation. Am I missing something?
I guess the next best solution would be to also have a pre-push hook
that performs the same checks again, just in case the bad code managed
to get past the pre-commit hook for some reason or other. This feels
very redundant, but I guess it would work well.
Any other suggestions?
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: pre-commit hook not run on conflict resolution during rebase
2015-05-28 19:38 pre-commit hook not run on conflict resolution during rebase Stefan Haller
@ 2015-05-28 20:03 ` Junio C Hamano
2015-05-29 7:25 ` Stefan Haller
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2015-05-28 20:03 UTC (permalink / raw)
To: Stefan Haller; +Cc: git
lists@haller-berlin.de (Stefan Haller) writes:
> When a rebase stops because of a conflict, and I edit the file to
> resolve the conflict and say "git rebase --continue", then the
> pre-commit hook doesn't run at that point,...
> From glancing through the githooks manpage, I couldn't see any other
> hook that would help in this situation. Am I missing something?
I do not think so. There may be some fallouts, like negatively
affecting folks who have been relying on the current behaviour, but
I do not see a fundamental reason why that hook should not trigger
there (it may not trigger in the current implementation, but I view
it as lack of need so far, not a deliberate omission).
> I guess the next best solution would be to also have a pre-push hook
> that performs the same checks again, just in case the bad code managed
> to get past the pre-commit hook for some reason or other. This feels
> very redundant, but I guess it would work well.
I'd say pre-receive would be the most sensible place to check things
like this. Some of your developers may not have pre-commit hook or
even run "commit --no-verify" to bypass the local check, and if you
have a central meeting place for the efforts by all your folks, that
is where you want to enforce the policy.
Checks done anywhere else are what are redundant, including the
pre-commit hook in individual developers. You can view them as a
way for individual developers to save their time---by choosing to
check locally, they make sure their commits do not trigger the
pre-receive hook at the central place.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: pre-commit hook not run on conflict resolution during rebase
2015-05-28 20:03 ` Junio C Hamano
@ 2015-05-29 7:25 ` Stefan Haller
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Haller @ 2015-05-29 7:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> wrote:
> lists@haller-berlin.de (Stefan Haller) writes:
>
> > I guess the next best solution would be to also have a pre-push hook
> > that performs the same checks again, just in case the bad code managed
> > to get past the pre-commit hook for some reason or other. This feels
> > very redundant, but I guess it would work well.
>
> I'd say pre-receive would be the most sensible place to check things
> like this.
Yes, I totally agree, and we used to have this setup when we were still
hosting our code in-house; with pre-receive doing the authorative
checks, and pre-commit being optional as a convenience for developers,
as you say.
Now we have moved most of our code to github, and you can't have
pre-receive hooks there, as far as I can tell. (I should have mentioned
that, sorry.)
To make up for that, we have put considerable effort into ensuring that
everyone on the team has up-to-date hooks locally, by installing them
automatically as part of our build system infrastructure.
In that light, do you agree that a pre-push hook is the best we can do
now to plug this hole?
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-05-29 7:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 19:38 pre-commit hook not run on conflict resolution during rebase Stefan Haller
2015-05-28 20:03 ` Junio C Hamano
2015-05-29 7:25 ` Stefan Haller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.