1425999328 kitty

Ups and Downs of Pull Request Flow - part 1

Photo by yomo13

We are often asked if our teams use feature branches and/or pull requests in our projects. When we say no, the next question is usually followed by some form of “don’t you guys care about quality?”. We do very much; quality is one of our primary concerns. We used pull requests and feature branches in the past, but found that in most cases, contrary to intuition, they lead to decreases in quality, not increases. PRs are a great tool to manage your codebase, however they introduce some horrible traps that many development teams tend to fall into - especially with commercial projects.

Pull request flow traps, sad stories.

Here are some of the pitfalls when branching a project:

Developers wait too long for code review

  • In a fast-paced project, your PR might be based on code that does not exist any more; even after a span of only two days.
  • You always work on “old” code. Rebasing from the master branch often won't help this problem - others still work on "old" code from your perspective, they don't have your changes in their work.

Developers do long-running branches without feedback

  • Early problems aren’t detected soon enough and late feedback is often too late to help.
  • Alternatively, you have awesome code but others don’t get to see or use it right away.

You limit the number of people that can accept the PR - CTO, team leaders, senior developers

  • Throw your CTO under the bus and your team is done.
  • Collective team knowledge is limited to the amount of people ‘accepting’ the code.

You can recognize if your team has fallen into one of these traps when your team members often ask if something is already merged (a very bad sign).

The “Refactoring” story

  • Developer A works on a feature for about a week.
  • Developer B in the same project does huge refactoring within the same week.
  • B finished first, PR accepted, refactored code on master.
  • Developer A pulls the changes to his branch. Week of work is now completely broken, as the feature is based on code that no longer exist, as a result of huge refactoring.

Result: a week of developers’ work has to be re-done. The PM is sad, the client is sad.

“A new developer joins the team” story

  • A new developer joins the team and works on feature A within 4 days.
  • An experienced developer does a feature PR review.
  • It turns out that the new dev did not base the implementation on things that already were in the project, and just created new stuff, as she/he did not understood the project yet.
  • The work could have been done twice as fast

Result: time loss and quality plummeting, nothing to be done at this point beside refactoring, which probably won’t be welcomed as implementation already took double the amount of time than it should have. Developers are sad, PM is sad, client is sad, and kittens are very sad.

“Can’t even remotely estimate” conversation

DEVELOPER: I’m going to start the SuperCrm integration ticket now! It’s just another Integration. Easy peasy - 2 days tops.
OTHER DEV: Can you wait a bit? I’m rewriting Integration now, it’s going to be super easy peasy to do your story once I’m done.
DEVELOPER: OK!
[2 days later]
DEVELOPER: I’m going to start SuperCrm ticket now!
OTHER DEV: Can you wait a bit? I’m still not done.
CLIENT: So it’s 4 days total now?
PROJECT MANAGER: :/
DEVELOPER: :/
[Other dev quit, lost connection]

Lost value

All those traps lead to time and work loss, and in turn, to quality decreases. But there is one more story that is the worst one here. This is the story of lost value.

NEW DEVELOPER: Is that branch important?
DEVELOPER: No it’s just my 2 week refactoring I did last year but never finished because I had to take care for that other thing...

‘Never finished / never merged’ branches and features in the repo are the manifestation how much value is lost in the process - as clear as it gets.

s3

Branch as sad as sad kitty (from a project we took over recently)

That's all for now. In the second part of this post you will find what our team does instead of branching - and how we keep our developers, clients and kittens happy.

On 13.01.2014 in Our Way and Dev