3

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?

  • Hi, Marcus. Your title question is probably too broad. Whether your branches should always be in a working state has been answered before on here a few times before -- but you are specifically asking about why it isn't discussed in many articles? (Isn't it stated or at least implied in all the ones you linked?) – Jeremy Hunt Nov 10 '19 at 3:37
  • @JeremyHunt: fixed. – Doc Brown Nov 10 '19 at 8:33
  • If your process allows for a non-working master, your process is wrong. – Pieter B Nov 11 '19 at 10:57
7

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:

| improve this answer | |
  • This is the assumption I have always had as well. But I wanted to find some sources confirming this. Thank you for a good and proper answer. – Marcus Ekström Nov 10 '19 at 22:39
  • @MarcusEkström: Trust your instincts. – Robert Harvey Nov 13 '19 at 16:03
2

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.
| improve this answer | |
0

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.

| improve this answer | |
  • 1
    "Release early, release often" exactly leads to the conclusion that the master needs to be in a buildable (and thus releaseable) state at all times. You're trying to conclude the opposite but it doesn't make much sense, a broken master cannot be released (successfully). – Flater Nov 18 '19 at 13:19
  • 1
    Your "panic mode" argument isn't bad (panic fixes can be needed in certain scenarios) but it also conflates the concept of a broken build with a build with missing features. These are two separate things. Missing features are not an inherent issue (assuming development coordination of course), but a broken build IS an inherent issue as it becomes a blocking issue for ALL features (since you can't build). – Flater Nov 18 '19 at 13:22
  • "There are organizations where risk is encouraged, and you are allowed to break the build to deliver" This also seems to forget what a broken build exactly is. Breaking changes != broken build. The former is good for making progress. The latter inherently prevents progress as no product can be released anymore and thus no progress can be made. – Flater Nov 18 '19 at 13:24
  • On the other hand, breaking production without necessarily breaking the build per se is probably a more grave error, but also one which e.g. Facebook seems to encourage. – tripleee Nov 18 '19 at 13:34
  • Sure, but that inherently requires e.g. Facebook to redefine what an "intact" master branch is, which makes the distinction moot for the purpose of the question at hand. A master branch which is not intact is one that does adhere to the expected state for the master branch to be in. Companies like Facebook simply define different expectations, rather than allowing the master branch to be in any state. – Flater Nov 18 '19 at 13:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.