Question

I've read a lot of useful articles about collaboration with git. Many do's and don'ts, etc. For example, Top ten pull request review mistakes and a nice article about git workflow The Perfect Code Review Process, which is a nice way of doing it. Quite similar to the famous A successful Git branching model. I also really like this one specifically about rules with git, or ethics if you wish The 11 Rules of GitLab Flow.

But what I don't understand is why barely any articles ever mention what I find to be the most important rule of them all. To always make sure your main branch (usually master), is still fully functioning. The worst thing I know is when I am working on my feature branch, I merge in master only to realize my branch stopped worked. Or to have developers being stalled from starting on a new feature because the master isn't functioning as it should. In my eyes, tests combined with code-reviewing should cover this. But I rarely see articles mentioning this specific rule.

So am I wrong, is keeping the master branch intact not so crucial for team development as I think? Did the authors of those articles forget to mention it?

Was it helpful?

Solution

Scanning through all the four articles you linked to, it seems they are written from a perspective where the rule to keep "master" intact is assumed to be so basic that it is not worth mentioning.

In fact, this rule does not only apply to "master". It applies to every branch which is shared for collaborative work between two or more people of the team, or a branch which is pushed to a build server. If, however, you are working alone on a certain feature branch, it is often not important to keep the code fully compilable and all unit tests in a non-failing state (though it may be a good idea, either, at least to try to get to that state as often as possible).

Why isn't it mentioned explicitly more often? I can only speculate, but my best guess here is: everyone who ever worked in a team and does not follow that rule finds out about it rather quickly by the feedback of the other teammates. If one checks in some code which disturbs others in their work, the latter persons will probably not be happy and tell the former to be more careful.

See also:

OTHER TIPS

To always make sure your main branch (usually master), is always fully functioning.

I agree with you.

As an external contractor who has to go through rigorous code reviews (3 people!) before my pull request is accepted, it is infuriating to see the company employees check in broken code that they never even once ran (e.g. blatant nullreferences - passes the build but clearly blows up) into the master. When I merge the master into my branch, suddenly I'm fixing their bugs for them since I can't fix the master for them, and the entire CI/CD pipeline is stuck until I get my pull request reviewed.

In my eyes, tests combined with code-reviewing should cover this.

The better solution (compared to what I'm dealing with) is to enfore that the master cannot be directly touched and can only be updated via pull requests.

However, it's never possible to 100% guarantee that there are no issue in the build. The build may succeed, but there may be runtime errors that simply weren't tested for. Tests can only cover that which they've been written to cover.

So it's always possible that an issue makes it into the master. And in these cases, it makes sense that developers (or at least the lead dev(s)) have access to the master to quick-fix issues that are holding up the CI/CD pipeline.

Can you force those fixes over a pull request? Sure. But the additional overhead for a fix that takes a few seconds is quite big and there are time where the pressure is too high to wait for that.

It makes sense for someone to have push access to the master to fix any breakages; but it shouldn't be commonplace for anyone to push directly to master.


That being said, while I'm aware this is slightly pedantic, there are fringe cases where the master needn't be in a buildable state:

  • When you're still setting up the initial framework (projects etc), it makes little sense to work in multiple branches as you'll have merge conflicts for days, and usually it's only one person setting up the empty application framework anyway.
  • When you're making a massive structural change and it has been agreed upon that the CI/CD pipeline is on hold until that change is through.
    • While it still sounds better to do this in a separate branch, consider that it makes little sense for others to continue implementing features on a codebase that soon will be dramatically changed. Git might not even be able to halfway figure out how to merge the feature with the new architecture.
    • However, if you incrementally push framework changes to the master (still broken), people can at least start developing features on the new parts of the architecture that you have already finished (even if the overall build isn't finished yet.
    • This is of course an exceptional circumstance; no one should ever want to change their architecture this dramatically while still actively developing.

At the peril of attracting downvotes, let me offer an alternate perspective.

I don't disagree with the idea that you should only push working code. I am merely offering an explanation.

At different stages of development, you have different objectives.

During early development, a common rule of thumb is "release early, release often." A good way to get feedback -- even for silly mistakes which break the build -- is to make your code available to collaborators, even when it might not be fully working. This definitely requires a mutual understanding that this particular project is a work in progress, and that the developer does not commit to keeping the build healthy. We note that this is a problem in any setting where you have CI/CD in place.

In "panic mode" development, maybe you are (somewhat) happy to push a security fix even if it breaks some other functionality. Again, there should be an understanding among collaborators that there are pesky priorities at play and that some crucial technical debt might need immediate attention after the core issue is solved.

There are organizations (like, famously, Facebook) where risk is encouraged, and you are allowed to break the build to deliver (presumably exciting and addictive) new functionality. Hopefully obviously, again, this is a situation where everybody needs to be aware of any such deviations from common software development norms.

The bottom line here really is that collaborators always need a well-understood and ideally well-documented agreement about what to expect from committed code and probably additional channels of communication in scenarios like these.

Licensed under: CC-BY-SA with attribution
scroll top