I think in a team with good ownership, enforcing formal reviews slows down a lot. But of course in a larger code base where no single engineer can understand all the effects a change might have, having a bit of knowledge sharing and 4 eyes enforced is often the better approach than yolo-ing it.
Then again I did build an SQL review tool for database access because I felt like "yolo-ing" production access was also not the way it should be. It's an interesting slider between full autonomy and strict review processes where each team needs to find their sweet spot: https://github.com/kviklet/kviklet/
There's nothing wrong with small, short-lived branches that can be quickly reviewed and merged into main.
That being said, I've been in a small team where the blessed style was to commit directly to main and do reviews "on demand". It quickly gets features deployed in a way that builds a lot of rot and debt into your project that people quickly lose a good understanding of. Then you just keep piling.
There's probably a way to get this done properly with a tiny team of very experienced and well-aligned developers; but that's usually not what you have in an environment that pushes this kind of extreme no-review-interpretation of trunk-based development.
Slow down, do reviews, avoid keeping branches open for more than a day.
I'd like main to always be ready for production, but that seems an elusive goal no matter what git workflow you use.
The best way to prevent complex merges does not depend on your git strategy, but on how modular you make your code. If a change requires changes to only a single file, and your files aren't too big, there's little chance of conflict. The more files need to be changed (often because the same thing needs to be declared in 4 different places), the bigger the chance of conflict. Same with larger files. Each file should have a single concern.
Code should live in main/master and be in a production environment as soon as possible, but it does not have to run for all, most or even any customers/clients from the start. You can gate it behind feature switches, preprocessor directives, global constants, whatever. As long as it has an active purpose and is being developed further, it is not dead. Ideally, it will have tests.
The result of merging back to main often is that other changes will be made in awareness of your new code, and all merging will happen organically and in small increments. You also get to develop complex features bit by bit, in a way that can be verified in a real environment in a controlled way. And everything can be deployed anytime, which should happen often.
Being able to verify incrementally and get quick feedback lowers the risk of otherwise releasing a hitherto completely untested large feature into the wild in one go.
This does not absolve you from making sure that your changes actually work as intended. That will be easier if you portion the work into smaller chunks that are easier to understand than a whole, complex, all-or-nothing feature.
Of course you will still have to make sure that your small chunks fit together and make up a good "whole". However, the idea is that you're doing this exact work anyway when developing a larger feature on the side, but you might as well cut it up to deliver in much smaller increments.
This approach has its own costs and drawbacks, but in my experience it often produces a substantial net-reduction in friction and accumulated complexity and risk, and it keeps things moving, which is a benefit on its own. This is from my perspective in backend development, and it will not apply universally, but widely. Essentially, whenever you can get away with it, you should strive to deliver like this.
I think this is very key, if the development style and the direction of the project is clear, much less review and alignment is necessary.
And also
> avoid keeping branches open for more than a day
Big +1 on that, fast reviews are extremely key. Most teams I have seen often took days or even weeks to merge branches though, often because you end up waiting too long for reviews in the first place. Or because of good old bike-shedding. But also because these code reviews often uncovered uncertainties that needed longer discussions.
However usually code is easy to change, so defaulting to "just merge it" and creating followup tasks is often the cheaper approach than infinite review cycles.
Once the code is merged, chances are it will not get changed Those follow-up tasks will be displaced by more pressing work that will keep piling onto a slightly unstable foundation, increasing the tilt over time.
There is an excluded middle between "no reviews" and "infinite review cycles": proper, timely and efficient reviews. They are worth investing the time to get right. They will start paying dividends months down the line, and boy will they keep paying.
This is not about trying to get things perfect from the get go, but to get them done right while you're there. "We'll fix it later" is not gonna happen, and is much more expensive than it initially seems.
I wish this was the "default" mindset everywhere, especially in those cases where you have that one colleague that loves to nitpick everything and doesn't see an issue with holding up both releases and wasting your time over menial pedantic stuff. It would be so much easier to merge working code and let it work, and keep those TODOs in the backlog (e.g. trash).
In a sane world, code review would be like:
1. Will this work and not break anything? We checked it, it's okay. There are no apparent critical or serious issues here.
2. Here's a list of stuff that you can change if you wish, here's why it might be an improvement.
3. Okay, we have some left-over nice to haves, let's keep track of those for later (or not) and merge.
It gets infinitely worse if you're working on 3 projects in parallel and the person wants long winded calls or starts nitpicking about naming, or wants things done exactly their way as if it's the only way (doubly worse if their way is actually worse by most metrics and tastes).> There's nothing wrong with small, short-lived branches that can be quickly reviewed and merged into main.
I would have called this "branch based development", personally.
https://www.atlassian.com/git/tutorials/comparing-workflows/...
But yeah, that’s what people generally settled on, naming wise.
The team was small, around 6 people, and the codebase was maybe medium sized (~500k LOC). There was no formal review process, instead it was up to each team member to ensure the quality of their own and others code. In practice I would read through all commits that came in the previous day while having my morning coffee. If there was some egregious I would talk to whoever made to commit to make discuss if something should change, but this was fairly rare.
Formal PR reviews were only ever really used for new members or for bigger/sketchy changes where someone wanted more eyes on it.
Because I ended up reading most commits, I ended up knowing how pretty much the entire codebase worked. It takes a while for this to develop, but the more you do it the better you get at it, especially in the context of a single codebase.
I think you're confusing workflows with commit history.
You can work with feature branches all you want, rebase them as you feel like it, and then do squash merges to main.
The likes of GitHub even have a button for this.
Assumption: above mentioned total noobs don't use git rebase -i or equivalent, everyone else does
In 25 years of professional development I’ve never really had a situation where the commits on a branch would have helped me understand what was going on a year ago when the work was done. That includes pretty big bits of project work.
I’d much rather have a trunk with commits at the granularity of features.
What has happened a whole lot though is the exact opposite.
I've had separate commits come in handy several times when `git blame`ing when working with people who actually described what changes were about in their commits (which, unlike comments, don't go out of date).
Coincidentally, every single squash-merge commit advocate I've had the unfortunate debate with was a regular practitioner of public tmp / tmp / try again / linter / tmp / fix / fix / haaaaaands commits.
Note that I'm not against squashing/history rewriting e.g rebase -i and stuff (which I'm a heavy user of so as to present sensible code aggregation reviewable per-commits), only squash-merge.
My professional experience contrasts with yours. I've even worked at a company where commit history and PRs were so central to understand and explain changes that PRs were even used as the authoritative sources on how to implement features and use frameworks.
It's less about reviewing commits from a year ago, than making change low-risk today. And small commits can easily be rolled back. The bigger the commit, the more likely rollback will be entangled.
It better to have partial features committed and in production and gated behind a feature flag, than risk living in some long-lived branch.
This constraint is usually enforced by code review.
On Github, the unit of code review is the PR.
Therefore, I prefer squashing.
Why would you not want to squash merges? It's one of three options offered by GitHub in their PR merge buttons.
They create a linear commit history, which is what you want when you have to audit changes.
> except to clean up messy mini-branches written by total noobs.
Nonsense. You get a messed up commit history as easily as when you create a PR in GitHub, and after team members merge their commits, you click on GitHub's "update branch" button.
You also get a messed up commit history if you merge a PR after someone else merged theirs.
You will always mess up your pristine commit history if you have more than one person posting and merging branches. With one notable exception: squashed commits.
Do you take their tagged release, fix it there, and then send them that branch release with the fix, or do you send them a fix on current main - you know, the main that is now a million releases ahead with multiple breaking changes? And what about all the intermediate release tags? Do you fix each one there too if they have the problem, or do you only update when customers on those releases start having that issue too?
And if you fix to their old tagged release which is incompatible from main, does this mean you have to do this fix twice i.e on their tagged release and also fix it for main? But what if this fix affects other people who are all on different branches too? Now… times this by 20 different customers all running different hardware and different branches with different problems :(
Maybe my comments are off topic, and don’t get me wrong - I prefer “trunk is releasable” motto, but I think maybe as an industry we should all come up with an Acid Test (like the only CSS Acid Tests) so we can through all these branching strategies into the ring
The answer is development branches are forbidden but releases still use a kind of branching approach.
When you make a release you use commit A from main, then development continues, commit B adds a feature, and maybe commit C fixes a serious bug.
You don't want to make a new release at C because it includes new non tested features, instead you cherry-pick fixes to your release, test the new release candidates, and release that when ready.
Development still happens in main however.
Another big tool to minimise these problems is to separate the concept of feature release from the concept of binary release. You don't have to make a true deployment to release new features or roll them back, just use a toggle switch.
If you build a software that you distribute so people can deploy it themselves (a library, a self-hostable solution, ...), then you most likely semantic versioning. In that case, the best model is to use what semantic release offers.
We just cherry-picked stuff back to release branches, if we needed a fix.
The triple mortal loop, comes that we have two versions of the product. One with the old no responsive frontend and other with a modern responsive frontend. And we need to release and develop the two versions for sometime, before the direction decides to kill the old no responsive version. So we end with 4 branches: release, release_rwd, develop and develop_rwd. If we fix something in release, we need to do a diamond merge : release to release_rwd, release to develop, release_rwd to develop_rwd and develop to develop_rwd
But probably the worst part was that when I started it was a loose organization where anyone could sneakily merge something into trunk. That changed quickly.
All work was done against a develop branch, and every two weeks, the admin would DELETE THE TRUNK and recreate it from a COPY of develop.
Every two weeks, we lost all history and context for changes in any given release. This was an effort to stop bugs coming back in merge regressions.
Can you guess how well this worked out for them?
And some people hate it… but, up to and including this story of random-assed deletions, scratch the surface of why and all you hear are self-inflicted harm through bad process.
“Git sucks butt because at OUR shop we start a new repo every few weeks with copy n paste and don’t understand partial checkouts and under-resource the primary host server and also people sneak code into production for funsies.”… … yeah, sounds like “Git” has problems…
I'm always surprised by how popular SVN still is though. I ported my old Sublime SVN plugin to VS Code years ago for fun, and I still get issues raised today (usually in Chinese, so it seems popular there). If you look at the VS Code marketplace, the top SVN extension has ~1.3m installs.
Bit of a tangent, but it probably wasn't a good idea to release an extension for something I had stopped using long before, particularly when you have to maintain it!
Seriously wish the stacked PR workflow would gain more traction outside of FAANG. Apart from the (somewhat pricey) Graphite offering, there's no standard UI for managing stacked PRs in the wild.
[1]: https://x.com/jaredpalmer/status/2019817235163074881?s=20
Gerrit looks ugly and is not very easy to use at first, but at least it's not slow like GitLab and GitHub.
[1]: https://lore.kernel.org/git/CAESOdVAspxUJKGAA58i0tvks4ZOfoGf...
It allows you to get feedback from customers very fast.
It allows you to improve the software very fast.
It allows you to react to the feedback you just got very fast.
Yes, its tricky! You need fast builds, that give you actionable feedback on whether you did a whoopsie.
Yes, it works for all sorts of things: regulated industries, incl finance, embedded systems, apps, websites, ...
Yes, you do need to rethink how changes happen, to look for ways to make that big change into multiple or even many smaller changes, this often has lots of unanticipated benefits.
Yes, it scales to very large deployments and quite large teams.
especially combined with monorepo
amount of time people spend updating dependencies between internal services and libraries in a pursuit of semver for now reason is just absurd
Prescribed workflows apply solutions from the wrong direction.
Determine what's impeding your organization, devise a solution.
The last guy I encountered who evangelized something like this had smart frames and a Lovecraft anthology prominently perched in the background of his Zoom shot.