Opened 7 months ago

Closed 6 months ago

Last modified 5 months ago

#30033 closed defect (implemented)

The pre-push hook should call the pre-commit hook on the final pushed commit

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: git-scripts
Cc: rl1987 Actual Points:
Parent ID: #29792 Points:
Reviewer: ahf Sponsor: Sponsor31-can

Description (last modified by teor)

Sometimes when I do a change, then merge it forward, the pre-commit hook doesn't pick up style errors. That's because the pre-commit hook uses the old version of our maint scripts, but then I merge the changes into a version with stricter maint scripts.

Can we call the pre-commit hook on every the final pushed commit during the pre-push hook?

Child Tickets

Change History (10)

comment:1 Changed 7 months ago by rl1987

Status: newneeds_review

comment:2 Changed 7 months ago by teor

Parent ID: #29792

comment:3 Changed 7 months ago by asn

Reviewer: ahf

comment:4 Changed 7 months ago by nickm

FWIW, I'm not sure it's a great idea to do this with _every_ commit -- perhaps we just want the final one. Otherwise we'll be rejecting branches that clean up something at the end, yeah?

comment:5 in reply to:  4 Changed 7 months ago by teor

Replying to nickm:

FWIW, I'm not sure it's a great idea to do this with _every_ commit -- perhaps we just want the final one. Otherwise we'll be rejecting branches that clean up something at the end, yeah?

Yes, I think you're right. (I thought about the same issue yesterday, but I forgot to write it down here.)

comment:6 Changed 7 months ago by teor

Description: modified (diff)
Summary: The pre-push hook should call the pre-commit hook on every commitThe pre-push hook should call the pre-commit hook on the final pushed commit

comment:7 Changed 6 months ago by ahf

Status: needs_reviewmerge_ready

Looks good. As far as I can tell it should only be called once now.

comment:8 Changed 6 months ago by teor

Milestone: Tor: unspecifiedTor: 0.4.1.x-final

comment:9 Changed 6 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged to master!

comment:10 Changed 5 months ago by teor

Sponsor: Sponsor31-can

These can be sponsor 31, if we want them to be.

Note: See TracTickets for help on using tickets.