> if the commits touch more than one of the modules (we have a monorepo with lots of individual services)

That seems wrong to me - a big advantage to having a monorepo is that when you need to make a change to how two services work together, you can do so with a single atomic commit.

I may not have been clear. You can merge a single commit to master (as a fast forward rebase) regardless of how many modules it touches. But you can't do it with more than one commit. It will refuse this because we don't want non squashed stuff on master and it was an easy enough hack to prevent that. Yes it's not fool proof but works well enough so far. It discourages feature branches too which we also want. All these scripts are part of the monorepo and if I need a feature branch for something I can easily exclude that branch in these checks. I've only done that once in the last few years.

This stuff is also very fluid and changed and improved as we go. If the easy solution turns out to not work well enough for enough common cases _then_ we spend more time making it better. Otherwise 'good enough' does the trick for us. YMMV depending on your company size and developer culture. E.g. you might have too many cowboys that just add exceptions all the time or remove the checks entirely. Nobody can help you there. Fire then or flee ;)

When a bug is found in production, it should usually be fixed by:

1. Make a commit supplying the test coverage that was evidently lacking.

2. Make a commit supplying the fix.

Everything you say is valid. However, what I just described is the simplest and clearest example of valuable multi-commit-per-ticket git history.

I myself do and encourage my guys to do multiple commits to checkpoint their work. Also for bug fixes as you describe, first commit is the test proving its broken. This should turn the build red. Then another commit with the fix which makes the build green. All of this on the branch. PR it like that. It's valuable to see this history.

Once the PR is approved and before merging to master, rebase abd squash. The build failing intermediate commit is no longer valuable.

> The build failing intermediate commit is no longer valuable.

At times what you say will be correct, but it's not at all obvious that what you say is always correct. Suppose that subsequently the authors of the fix in question have a horrible realization that there is something wrong with their fix, and furthermore they are panicking. Now, as a helpful colleague introducing some calm to the situation, wouldn't it be reasonable to recommend that they go back to the state of the codebase where (a) the original bug is present, and (b) it is demonstrated by a test?

But with your squash, no such state exists on main[0]. To my mind it feels a bit janky to have to check out some non-master commit from a feature branch to go to that codebase state.

--

[0] I am not an ideologue but I do kind of like main over master as it is shorter and clearer.

I usually go with what makes sense in most situations instead of complicating my life with stuff that may, perhaps, one day, if the moon is full and the stars are misaligned not work perfectly.

That said, I do not need to have an individual commit to do what you are saying. In fact our default is to remove all branches after the merge to master. But each fix is just one commit anyway. So the state you describe is actually just the commit on master that is before the fix. No need for finding old branches and finding the commit that has the test only.

The thing with the tests is also that this is an ideal scenario. Not everyone does this. I don't always do it either or can't do it. But I strive to. So relying on it would be bad. To do what you said regardless of whether there was a first commit with a test or not I simply need that one squashed commit, which is very likely very small and create a patch (simple diff of that commit and the previous commit meaning) and then remove all changes that aren't tests and apply this to the pre-fix commit. This is very likely very easy because tests are in separate folders and it's a small patch file. Probably takes less than a minute to do. Will not always work 100% but good enough in many cases and I can count on one hand how often this would have been needed in my entire career.

(in fact I did this for a similar thing recently. I needed to apply some of the code from a spike. I created the patch, deleted unwanted stuff from the patch file and then applied it. Easiest ticket I did in a while ;))

Basically: 'git diff <prevCommitHash> > abcd.patch' then 'git checkout <prevCommitHash>'. 'vi abcd. patch' aaand 'patch -p1 < abcd.patch'

Re: Master. That's the name of the branch as per git default. Not much I can do there ;)

There's negative value in having a commit in the history which turns the build red; it should be removed, but the requirement that the branch is squashed onto master doesn't follow.

It'd be a perfectly valid alternative in some cases to clean the commit history and rebase a number of atomic commits onto master if it would make the history more readable.

Suppose

(a) You suddenly realize that the production code in the main (not deployed) branch is incorrect

(b) There is no test coverage for the bug

(c) You have time to commit a test reproducing the bug, but a fix will take days.

In this situation you think there is negative value in committing a failing test that will make the build go red? Perhaps you were forgetting that sometimes commits alter production code paths and sometimes they alter test code?

If you turn the main branch red, you affect the workflows of all developers who pull until you turn the build green again. It may also affect git bisect depending on the bisect workflow used.

The ideal would be to revert the change if known, or disable deployment on the branch until the issue is fixed with an ignored test*. I feel a workflow where you explicitly break the build [incl. tests] when a bug is discovered isn't ideal.

I agree. This is not how I meant it (OP here again).

Our master is alway kept green. If master is not green we are dead in the water. If something happens and master is not green we can not deploy any fixes.

As you say we either revert the breaking commit and push that out fast or we deploy the last known good commit. This may just be HEAD^ if there was only one new commit going out. It might also be several commits ago. Depends on what got merged recently and made it into that deploy.

On bisecting: we keep the actual images for some time. So if you don't need to bisect something rather old, then you don't need to build anything to bisect. Only deploy and check whether you had a good or bad commit. A deploy takes less than a minute usually vs a ~10 minute build.

> You will not be able to merge a PR that tries to get more than one commit onto master

What value does the merge commit in the PR provide if a PR only has a single commit? Couldn't you essentially halve the number of commits in the commit history by amending the single PR commit message to contain the text that the merge commit will contain anyway?

In case it wasn't clear from my post, I am very all over the place with the word 'merge'. I usually use it to describe the concept. We don't ever have actual merge commits. We merge by rebasing and that then makes the 'merge' just a fast forward.