captainhookphp / captainhook

CaptainHook is a very flexible git hook manager for software developers that makes sharing git hooks with your team a breeze.
http://captainhook.info
MIT License
1.01k stars 87 forks source link

[feature] {$CHANGED_FILES} in pre-push #202

Closed CircleCode closed 1 year ago

CircleCode commented 1 year ago

As this is a good practice to keep commit easy and smooth, I try to keep my pre-commit hooks as fast as possible, so I limit what will run. However, I would like to provide feedback to developers faster than the CI, and for this purpose, I would like to run some additional tasks on pushed files.

The definition of pushed files being a bit ambiguous, I would define them as the output of git diff --stat --name-only origin/master...HEAD (<reference> being the name of the remote branch by default, but being overwritable in the placeholder. In my case, I want reference to be origin/master which is fast forward only).

Of course, this could be done in a custom script, but then it would defeat the purpose of simple config in captainhook.json.

Ideally, {$CHANGED_FILES} would have the same modifiers as {$STAGED_FILES} plus a |base:<reference>.

Would you accept a contribution in this direction? And do you think sebastianfeldmann/git already has everything required?

sebastianfeldmann commented 1 year ago

Adding something like {$CHANGED_FILES} sounds like a great idea.

Not sure everything is already there but BlockFixupAndSquashCommits should be a good place to start.

I just looked at the current Placeholder implementations. Currently they don't have access to IO but they would need that in order to get the refs that will be pushed. You can not just assume that it will be master. Maybe you want to push to some feature branch or multiple branches at the same time. But as I mentioned BlockFixupAndSquashCommits is a good stating point to understand how to gather the necessary information.

I'm not getting the |base:<referecnce> idea, what would that modifier do?

But yes, definitely open for something like this.

CircleCode commented 1 year ago

I'm not getting the |base:<referecnce> idea, what would that modifier do?

As I said, the notion of changed files can be a bit ambiguous. Changed against what?. In my mind, it means “The files you explicitely chnaged on your branch since you forked from the branch this will be merged into an the end”, or said differently in my workflow: “the files you changed sinced you forked from origin/master”. However, as you said, some will want another reference branch, like “origin/prod”, or even the remote reference being pushed to.

The {$CHANGED_FILES}|base:<referecnce> would allow such customizations:

sebastianfeldmann commented 1 year ago

You can not just define what you want your base to be. If you are pushing to different branches this will fail.

You have to determine on push what the current local and remote states are.

Luckily git provides that information as stdIn to the pre-push hook. You can check BlockFixupAndSquashCommits for more details.

The most important thing is that you can push multiple branches at the same time. So you receive potentially multiple LOCAL_STATE<-->REMOTE_STATE combinations.

sebastianfeldmann commented 1 year ago

I started working on this, hopefully I have something to test tonight

CircleCode commented 1 year ago

In the case several references are pushed, with different changed files, what will be the value of this variable?

sebastianfeldmann commented 1 year ago

I my opinion it should contain all files, so if you push file A and B to branch X and file C and D to branch Y the list should contain A, B, C, D

sebastianfeldmann commented 1 year ago

Fixed with 5.15.4