Safer branching

19 points by gitdude 9 years ago | 11 comments

guptaneil 9 years ago |

If your company is large enough that somebody can be stuck on a problem somebody else knows the answer to (i.e. pretty much any company with more than ~10 people), you should not be committing directly to master. All commits should be submitted via a pull request that passes your CI and code review (and ideally QA review too) before being merged to master.

kannonboy 9 years ago | |

Absolutely. An alternative title for the comic would be "For the love of branch permissions". The second developer in the comic is doing the right thing by creating a feature branch.

However even in a strict branching workflow, there's still a chance you'll have genuine integration failures when two branches are merged, even if they independently pass the tests.

sytse 9 years ago | | |

I totally agree it should have been a merge request and the master branch should be protected so novices can't merge to it.

There is a chance that the merge itself breaks the build. I must say that in practise we don't often see this. Maybe because we keep our branches small. Other people seem to be more aware of this problem https://gitlab.com/gitlab-org/gitlab-ce/issues/4176

dmitry-k 9 years ago | |

Couldn't agree more. In RhodeCode (https://rhodecode.com) we use Pull Requests with a voting system. Interested reviewers are added automatically, depending on the repository and the changes being made.

josho 9 years ago |

Nope.

My command line git workflow should not require working through a GUI.

Now, if this was added as some git hooks, enforcing the policy from both the command line and GUI then I'd be impressed (and excited).

alblue 9 years ago | |

That's pretty much what Gerrit does. Works with any git client, since it's just dealing with pushes, and can. E configured to merge automatically or with a manual nudge once the tests pass. And you can do this all through the command line; no GUI needed.

kannonboy 9 years ago | |

Comic author here. For the command-line I use a post-checkout hook to display the build status of a commit whenever I switch commits:

https://bitbucket.org/tpettersen/bitbucket-build-status-hook

That way if I update master before starting on a new feature, I get a notification if the tip of master is broken (or still building).

eridius 9 years ago | |

What would be nice is automatically attaching git-notes to commits indicating whether or not they passed CI. That said, I haven't looked into git-notes for a few years, so I'm not sure if they ever made it easy to sync them without a bunch of manual configuration.

jmakeig 9 years ago |

This is a cultural problem not a technical one. The social contract in a dev team should strictly enforce no cowboy commits to master…ever. If Bob thinks that’s OK, there are more issues than a UI warning is going to solve.

winteriscoming 9 years ago |

I felt it was an odd post for using as example a workflow which is common and then in the end trying to depict as something the developer has to be blamed for, for not using a UI for branching out.

Furthermore, using their UI doesn't guarantee that pipelines have been setup to test the commits in the repo. The article actually is about bitbucket pipelines, but hardly illustrates any of it.

I hardly use the web UI of remote repos unless I have to submit or review PRs. Especially the bitbucket UI which I find lacking intuitiveness when you start navigating the repos.

rkeene2 9 years ago |

My automated build/test server adds a tag (build-pass) to commits that pass the build and another tag (tests-pass) to builds that pass all tests.

So, instead of branching from trunk I would branch from tests-pass and that would be guaranteed to have been tested and pass, e.g.,:

$ fossil branch new my-new-feature tests-pass

I even added some CSS to make this look fancy: http://www.rkeene.org/viewer/tmp/fossil-with-tests.png.htm