craft a git pre push hook that declines to receive fixup! commits?

Every so often I see nickm push a commit to tor's main git that is a fixup! commit. It's clear that he meant to squash them before the push but didn't.

Is there some one-liner or something we can put in a git hook that notices these unintended commits and tells us to fix them before pushing?

changed milestone to %Tor: 0.4.0.x-final

added component::core tor/tor milestone::Tor: 0.4.0.x-final owner::rl1987 priority::medium resolution::implemented reviewer::dgoulet severity::normal status::closed technical-debt type::defect labels

+1 on this idea. It will help our history look tidier.

It seems likely that we'll need to involve the git-rw.tpo admins to install it, but I guess we might want to figure out details of what we want first?

Trac:
Cc: N/A to catalyst

Let's make sure the hook also includes squash! and the other directives.

Since this is a process improvement task, I am tentatively putting it in 0.3.6, marked technical-debt.

Trac:
Milestone: N/A to Tor: 0.3.6.x-final
Keywords: N/A deleted, technical-debt added

Tor 0.3.6.x has been renamed to 0.4.0.x.

Trac:
Milestone: Tor: 0.3.6.x-final to Tor: 0.4.0.x-final

https://github.com/torproject/tor/pull/530

Trac:
Status: new to needs_review

This works great for me.

After a discussion on IRC with rl1987, the changes were made that the git hook only considers master, release-* and maint-* branches. It won't apply to the other branches.

I'm also fine having it named pre-push in maint/ but there is also a possible argument to identify it has a git hook like pre-push.git-hook or something around those lines. I'll leave it to nickm to decide on that.

Trac:
Reviewer: N/A to dgoulet
Status: needs_review to merge_ready

I'd like the script to include directions on how to install it.

I like pre-push.git-hook as a name.

Question -- will this stop somebody from pushing the existing master branch (which has some errant fixup! commits) to a new repository?

Replying to nickm:

I'd like the script to include directions on how to install it.

It should be very straight forward: "Copy file into .git/hooks/ and name it pre-push" :).

I like pre-push.git-hook as a name.

Question -- will this stop somebody from pushing the existing master branch (which has some errant fixup! commits) to a new repository?

Yes it will but for that you can use --no-verify switch. So you can push all branches before installing the hook and be done with it :).

Okay. Should we have the script tell you about --no-verify in that case? Let's assume that the user will periodically forget how to use git.

Replying to nickm:

Okay. Should we have the script tell you about --no-verify in that case? Let's assume that the user will periodically forget how to use git.

Yup agree it could output it in the error message. So if I summarize, we need three changes:

1. Rename it to pre-push.git-hook in scripts/maint/
2. Add an indication to the --no-verify option to the error.
3. Comment at the start of the file that tells you how to install the hook.

Trac:
Status: merge_ready to needs_revision

Trac:
Status: needs_revision to accepted
Owner: N/A to rl1987

Trac:
Status: accepted to needs_review

lgtm;

Trac:
Status: needs_review to merge_ready

Merged, installed, successfully pushed!

Trac:
Resolution: N/A to implemented
Status: merge_ready to closed

closed

moved to tpo/core/tor#27993 (closed)