Code review and trunk based development

How, where, and when do you do Code Review in a Trunk based development setup?

By trunk based development I am talking about a specific approach to Git misuse common among macho junior devs with matching T-shirt's that says "only commit to master". We think we're cooler than than the other "grownups" using GitFlow or other overly complex systems. We do things with no rules. Agile! Anarchy! The invisible hand of the CI/CD Gods decides what floats and what goes.

Sure, at some times we will have something that resembles a "feature" branch, which offers convenience functions such as CI/CD done off the laptop, and GitHub and alike offers convenient code review of a blob of code before it lands on master. We use it more like a "remote stash" feature than anything else. And at some point in time we actually started using grown-up techniques like code review before we merged our "stash" of code into master. And we kinda liked it too since it really didn't slow us down much and it gave a way to talk about code.

But a lot of code - most of it we should hope - goes straight into master^Wtrunk. And that stuff sees no code review.

Where should we do code review? Where, when, and how do we learn from each other by discussing, reading our own and other devs code?

I have a radical idea. Well, incoherent thoughts that could lead to an idea.

We all merge to master. Independently. Optionally, we use our own private branches. Or work in small teams on branches. It does not matter. Merge^commit to master and have the CI/CD gods punish you hard if the code doesn't fulfill the flaky and spotty test suite.

However, the master branch is not production. master is merged to another repo or another branch and we do that merge together. If there are something we think is too stinky to be merged upstream we block that.

Or? Isn't this just dumb? How will this actually work?

Prod should be in sync with master. They cannot be allowed to drift too much can they? That sounds like a really bad idea if they did. And how do we know that the subset of commits actually works? Another really bad idea.

The only way to manage this realistically is to accept master wholesale into production. It is already tested. We are not cherry-picking or solving a complicated merge job here. What we are doing is a code-review. Together as a team. If we see something that is obviously going to blow up we hold off the merge --ff-only until it can be addressed. As early as tomorrow maybe, if we're doing these stuffs daily instead of the useless stand-ups people seem to insist on.

But the comments, all that bad code, that works, but just is wrong on so many levels? Who will fix that? The same developer that wrote it?

Maybe. Maybe not. They might think its great code. If you think it is so bad it does not deserve to be in production you have to make the effort yourself and fix it.

And then you as a team sit down (or stand up) the next time and integrate to prod and you review the code. And hopefully there is a big "aha" in the team. "Look how that code shines now! That was obviously a much better way to solve the problem! Fantastic!" Everybody learned from leadership and the work.

Or, more realistically, you tried to implement the feature you demanded the other dev should fix. It was obvious that they were not going to fix it after their "yeah, whatever man the code works" comment. But then when you tried to make your idea work, it just didn't work out in the way you thought it would so you just throw your high and mighty "feature branch" under the buss. Or you take the opportunity to show the feature branch diff with master and say "I tried implementing my idea I had last week, and you know what, it wasn't that great of an idea after all". Damn 5/5 leadership scored, again! But you did waste a week of development on your ego-branch so... maybe better to throw the code under the buss anyway.

What's your thinking - code review and trunk based development - do you have any good or bad ideas to share?

Update: If you are a Purist and think the idea of having a Production branch is disturbingly impure - consider this alternative. Create a PR from master - 1 day* to master and diff that in a PR. And then throw the PR and the branch away after review.

*) or whenever the last review was conducted.