While you make a compelling argument, I have to disagree with the notion that removing protections from the trunk is a feasible approach when collaborating as a team.
My reasoning is two-fold:
It’s a Quality control. Yes, you could just commit a typo fix to master quickly, but what if you introduce a new bug accidentally? Pull requests and peer review help build confidence in the changes being committed.
It’s not compatible with some engineering cultures. Our culture draws heavily from the principles laid out in the book Accelerate. Removing the trunk protection and allowing commits directly would begin to negatively affect performance metrics our organization maintains that ensure consistency, predictability, and stability of our releases and their cadence.
I’ve worked in collaborative teams ranging from 5 devs to 50, and in every instance we’ve kept branch protections to avoid accidents. It may be extra work to undergo the process, but it saves time in the end by avoiding unnecessary emergencies to unblock your release workflow.
That may be your experience, but I have the opposite. Forcing merge requests is:
annoying (wtf, I just corrected a typo in the comment!)
slow (damn, colleagues are gone, this will only be merged after the weekend)
extra work (damn, now there are merge conflicts again)
dragging down (code reviews is either very superficial or time consuming, sometimes leading to long fruitless discussion about style opinions)
is not safer (especially on large code bases, tests would catch undesired side effects that code reviews would likely miss)
has no sense of urgency (damn, there is a 0-day critical vulnerability in a dependency, I can't bump it directly! Let's wake up a colleague from bed to do it before everything burns!)
it breaks the flow (your mental focused state)
the version history is way more complex
The drawbacks in practice are numerous and the process mostly an annoying waste of time IMHO.
Please note that I'm not against merge requests themselves. I would just not force it. If it's a senior dev familiar with the code doing a small fix, let him do it directly. If it's a newcomer unsure of a large refactoring, of course let him do a merge request and review it. IMHO leaving both options on the table leads to a more productive outcome.
Jon is a self-taught programmer, started in video games but now does web development. He follows principles, argues for scientific software development, and does not like writing in the 3rd person.
@dagnelies all your points are exactly how I feel too, nice to not be alone 😅 It's all about options for me too, I find tremendous power in keeping a process flexible.
I wanted to also add the option of post-merge requests, where I merge in a change without review and then later invite a colleague over to go through my change. That can also be a suitable way of working in some contexts.
While forcing merge requests might be annoying for you - I would question why?
The act of raising a review isn't problematic at all, and the act of logging in, reading and pressing a green button isn't problematic either. So what annoys you about it?
Speed (colleagues not being available) is fine, sometimes we should slow down.
If you experience merge conflicts, that tells me that you didn't pull the latest main into your branch before raising the PR. That's a quality concern all of it's own, you're not contributing to the HEAD of main.
Having fruitless discussions is a culture issue - our choice is that PRs exist (but are optional in some cases), and they should be merged if the code is functional. If there's a debate to be had, open a new tech debt ticket and we can talk about it in there.
If you think that tests will catch all bugs, and therefore tests should replace reviews - oh man, I have a bridge to sell you...
Ugency, is one point where we agree - a critical hotfix, Production on fire - should not go via a PR. There are other ways to review those.
I don't see how a PR breaks flow - as a developer, you raise the PR and move on to your next task. I bunch up PRs for reviews and do them while I'm drinking coffee in a morning (again, we should take the time to slow down, IMO).
My version history looks rather simple, with our optional PRs. Most work goes via PR, some does not. I guess the secret there, is in the commit messages.
I guess I just experienced the pain of our recent enterprise-wide policy of creating branches for each tiny bug/feature associated with MRs/reviews. It simply adds beraucratic burden and leads to all the outlined problems. The conflicts are also not there initially, they occur due to delayed reviews. Aslo, sometimes, you must wait until something is merged before you can "build upon it". It just slows down everything and feels unnecessary most of the time. Like I said, for complex stuff or unfamiliar stuff, MRs and reviews definitely make sense and should be done. On the other hand, blindly enforcing for each tiny detail is IMHO just an unnecessary burden. Due to all the merging happening accross the board, the version history is also a lot less linear, and if someone merged it wrongly, more difficult to fix.
I manage a small team, and we look after almost 60 different components.
All of your problems can be solved, if there's a wider desire in the company to change those situations.
For what it's worth - I probably agree with you - we do a hybrid approach of "whatever makes the most sense for the situation we're in" - sometimes that's a PR, sometimes it's a hotfix, sometimes I'll just hop directly into the Production system to tweak something.
I suggest you speak with your development management, and with your PMO, along the lines of "hey, this recent policy feels like it will hurt delivery timelines, has X, Y and Z been considered already?"
Jon is a self-taught programmer, started in video games but now does web development. He follows principles, argues for scientific software development, and does not like writing in the 3rd person.
Hi Joe, valid points, and of course great to hear teams can weigh up the pros and cons and still find it worth having the branch protection.
Sounds like I've been in smaller teams than you (certainly not 50! 🤯😅), where main-commit accidents were rare and always short-lived so it never materialized as a tech-problem for us. I'd be a fan of trying an experiment to measure the with-and-without effect, if a team was curious to enable or disable branch protections
I would really love to read more about your performance metrics, it sounds like you've attempted to automate the DORA numbers? I've also experimented with that but didn't find a need to enforce pull-requests (TBH this feels like a topic worthy of a whole dedicated blogpost so it doesn't just disappear in the replies)
Re your #1 - if your concern is that another bug might creep in, then I would question if you trust people to check their work before they push a commit.
I personally use an IDE and a separate git GUI for that very reason.
And #2 is just a company decision, and could be changed, if wanted.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
While you make a compelling argument, I have to disagree with the notion that removing protections from the trunk is a feasible approach when collaborating as a team.
My reasoning is two-fold:
It’s a Quality control. Yes, you could just commit a typo fix to master quickly, but what if you introduce a new bug accidentally? Pull requests and peer review help build confidence in the changes being committed.
It’s not compatible with some engineering cultures. Our culture draws heavily from the principles laid out in the book Accelerate. Removing the trunk protection and allowing commits directly would begin to negatively affect performance metrics our organization maintains that ensure consistency, predictability, and stability of our releases and their cadence.
I’ve worked in collaborative teams ranging from 5 devs to 50, and in every instance we’ve kept branch protections to avoid accidents. It may be extra work to undergo the process, but it saves time in the end by avoiding unnecessary emergencies to unblock your release workflow.
That may be your experience, but I have the opposite. Forcing merge requests is:
The drawbacks in practice are numerous and the process mostly an annoying waste of time IMHO.
Please note that I'm not against merge requests themselves. I would just not force it. If it's a senior dev familiar with the code doing a small fix, let him do it directly. If it's a newcomer unsure of a large refactoring, of course let him do a merge request and review it. IMHO leaving both options on the table leads to a more productive outcome.
@dagnelies all your points are exactly how I feel too, nice to not be alone 😅 It's all about options for me too, I find tremendous power in keeping a process flexible.
I wanted to also add the option of post-merge requests, where I merge in a change without review and then later invite a colleague over to go through my change. That can also be a suitable way of working in some contexts.
While forcing merge requests might be annoying for you - I would question why?
The act of raising a review isn't problematic at all, and the act of logging in, reading and pressing a green button isn't problematic either. So what annoys you about it?
Speed (colleagues not being available) is fine, sometimes we should slow down.
If you experience merge conflicts, that tells me that you didn't pull the latest main into your branch before raising the PR. That's a quality concern all of it's own, you're not contributing to the HEAD of main.
Having fruitless discussions is a culture issue - our choice is that PRs exist (but are optional in some cases), and they should be merged if the code is functional. If there's a debate to be had, open a new tech debt ticket and we can talk about it in there.
If you think that tests will catch all bugs, and therefore tests should replace reviews - oh man, I have a bridge to sell you...
Ugency, is one point where we agree - a critical hotfix, Production on fire - should not go via a PR. There are other ways to review those.
I don't see how a PR breaks flow - as a developer, you raise the PR and move on to your next task. I bunch up PRs for reviews and do them while I'm drinking coffee in a morning (again, we should take the time to slow down, IMO).
My version history looks rather simple, with our optional PRs. Most work goes via PR, some does not. I guess the secret there, is in the commit messages.
I guess I just experienced the pain of our recent enterprise-wide policy of creating branches for each tiny bug/feature associated with MRs/reviews. It simply adds beraucratic burden and leads to all the outlined problems. The conflicts are also not there initially, they occur due to delayed reviews. Aslo, sometimes, you must wait until something is merged before you can "build upon it". It just slows down everything and feels unnecessary most of the time. Like I said, for complex stuff or unfamiliar stuff, MRs and reviews definitely make sense and should be done. On the other hand, blindly enforcing for each tiny detail is IMHO just an unnecessary burden. Due to all the merging happening accross the board, the version history is also a lot less linear, and if someone merged it wrongly, more difficult to fix.
I manage a small team, and we look after almost 60 different components.
All of your problems can be solved, if there's a wider desire in the company to change those situations.
For what it's worth - I probably agree with you - we do a hybrid approach of "whatever makes the most sense for the situation we're in" - sometimes that's a PR, sometimes it's a hotfix, sometimes I'll just hop directly into the Production system to tweak something.
I suggest you speak with your development management, and with your PMO, along the lines of "hey, this recent policy feels like it will hurt delivery timelines, has X, Y and Z been considered already?"
Hi Joe, valid points, and of course great to hear teams can weigh up the pros and cons and still find it worth having the branch protection.
Sounds like I've been in smaller teams than you (certainly not 50! 🤯😅), where main-commit accidents were rare and always short-lived so it never materialized as a tech-problem for us. I'd be a fan of trying an experiment to measure the with-and-without effect, if a team was curious to enable or disable branch protections
I would really love to read more about your performance metrics, it sounds like you've attempted to automate the DORA numbers? I've also experimented with that but didn't find a need to enforce pull-requests (TBH this feels like a topic worthy of a whole dedicated blogpost so it doesn't just disappear in the replies)
Re your #1 - if your concern is that another bug might creep in, then I would question if you trust people to check their work before they push a commit.
I personally use an IDE and a separate git GUI for that very reason.
And #2 is just a company decision, and could be changed, if wanted.