15

What is the advantage of using pull requests instead of simply merging a branch into master without one? Particularly on a team where all developers have full access to master.

  • 1
    Pull requests allow the project manager to decide if they want the branch merged into master or not. – Robert Harvey Aug 31 '16 at 16:56
  • In practice, if all developers have access to master, does it make a difference? – Goose Aug 31 '16 at 16:58
  • 2
    @Goose Code review? – nanny Aug 31 '16 at 17:00
  • 4
    We don't use Pull Requests at our shop. My understanding of Pull Requests is that they are mostly used on Github, where you have a public, open-source project published. As the project manager of such a project, rather than giving the entire world free rein over your project to make arbitrary (and potentially harmful) changes, you instead require people to submit their changes in the form of pull requests, so that you can review their changes before you merge them yourself into the master branch. – Robert Harvey Aug 31 '16 at 17:07
  • 4
    Because it's the DVCS way to never do in one simple step what you can do in 3 or 4 complicated ones – Mason Wheeler Aug 31 '16 at 17:25
23

Pull requests provide for checks and balances, even if anyone can push to master.

The biggest advantage is that they provide an opportunity for code review. The person that is responsible for performing the pull can look at the code and tests and make sure that they meet any kind of guidelines that the organization or team has. There are also other reasons for code review - education, finding defects or enhancements, cross-training the team on the system, giving testers a white-box view of the system.

If the person that performs the pull is familiar with the architecture of the system, then they can make sure that the changes fit with the architectural vision of the system, especially if the entire team may not have the long-term vision.

Developing a habit of using pull requests may also help your team if you decide in the future that the whole team shouldn't have access to master. If your team grows larger, and especially if you have team members who are new to the product and/or new to Git, not giving them access to master can be safer for product integrity.

5

Having done both feature branching and forks + pull requests I think that pull requests offer little advantage when you all are developing in the same team or company

They do offer a good mechanism and interface for code review, but also complicate and slow down the whole 'getting stuff finished' process. Especially if you have many small features, each waiting on review, merging and then all the others being merged with master again to pull the changes etc. If you have large features though then these become difficult to review, so you are caught in a bind.

Having said that you can do pull requests between branches on the same repo. You don't have to fork, or have different permissions.

Additionally you have to consider you entire methodology and workflow. Do you also have a ticketing system, CI, automated acceptance tests etc? Do your code reviews provide a vital single check before codes goes live, or are they just rubber stamp exercises which are made redundant by other checks in your workflow?

4

There is an observation called Conway's Law which states:

organizations which design systems ... are constrained to produce designs which are copies of the communication structures of these organizations.

What does this have to do with pull requests? Pull requests are a major communication channel at a critical junction for your code. They provide an opportunity for review, automated testing, and improvement before the code moves on to the next stages of testing and production, where those changes are much more difficult to back out, and waste a lot more time of a lot more people.

Likewise, Conway's Law suggests if you want to have a microservice architecture with cleanly separated areas of autonomous responsibility, and well-defined interfaces, then your organization's communication channels should reflect the architecture you wish to achieve. That means small teams of 5-10 people should have direct commit access to any given microservice, and anyone outside that team should be required to go through a pull request. This ensures the people most familiar with a microservice are the ones reviewing and advising about it.

When you have a large organization with everyone having direct commit access to everywhere, your communication channels of least resistance are setting you up to produce a big ball of mud architecture.

Pull requests only feel like a burden if you don't trade off anything in return. I've worked in environments where I can't get anything done for a week because the build is always broken, and I've worked in environments where someone submits a pull request and I don't even have to review it because they broke the CI build, and I tell you, they are worth every second of effort.

1

Karl Bielefeldt is exactly right. I would add: it's all about quality.

Many (most?) shops have no formal processes in place to govern development, which results in: "I've worked in environments where I can't get anything done for a week because the build is always broken, and I've worked in environments where someone submits a pull request and I don't even have to review it because they broke the CI build, and I tell you, they are worth every second of effort."

It really IS worth the effort.

  • Thank you for your comment. I'm not sure why to apply this as an answer to the question. – Goose Nov 29 '16 at 21:24
  • This is the only answer mentioning pre-merge CI. – Basilevs Nov 30 '16 at 3:26
0

We use pull requests for code review - no code should be merged into the main development branch (normally "develop" in our case, but sometimes "master") without having been through a pull request. We don't hard enforce this with repository controls, but that's because we don't have to - our developers are mature enough not to abuse the process.

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.