(no title)
jascha_eng | 8 days ago
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/
dxdm|8 days ago
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.
mcv|7 days ago
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.
jascha_eng|8 days ago
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.
reverius42|8 days ago
> 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.
roggenilsson|8 days ago
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.
maccard|8 days ago
div3rs3|8 days ago