I came to read this article on "Why your team doesn't need to use pull requests", I felt I needed to write a response. There are a few points I hope summarize the article well even with my bias.

• You have a team you trust and follows the conversations.
• Pull request reviews take longer than 30 min to turn around.
• Pull requests are not continuous integration.

Continuous Integration

I think the biggest mistake made is to suggest that CI is an alternative to pull requests. So let me define Continuous Integration.

CI is the process by which developed work is combined with the other development frequently. This movement has progressed and is now usually accompanied by a suite of tests. We will continue to include an automatic test execution as part of our use of the term CI.

Developing on Branches

I personally believe that commits are very important for communication and understanding. You can read more on my original series.

Git is a Communication tool

Jesse Phillips ・ Dec 12 '18 ・ 2 min read

#git

The key to CI on branches is

• Pull in mainline, with merge or rebase
• Small changes
• Frequently merge into mainline

In the article the issue begins at the point where the Pull Request has been created. Here is where the timeline starts and where continuous integration ends when using pull requests.

The turn around time is critical for maintaining reliable CI results.

Running Tests

Ideally your CI system is performing the same tests for your merge request as is done in the mainline branch.

Your code reviews should be occurring with accurate test results with main line recently integrated. As other work is pulled in, the integrated tests can be out dated.

The article is completely correct on the problems with pull requests.

• Stagnant pull requests are a liability.

Now that we have clarity in working with branches, let's look at the pull request process.

Pull Request Discipline

You need to have a team willing and ready to pull in changes quickly. The team can not give preference, ignoring previous requests for more interesting ones, but the PR may not be ready. PR that isn't ready is a liability.

Small

Your Pull Requests should be small and frequent. This does not mean every commit gets a pull request. But if you have completed something, a PR is reasonable for a single commit.

• Commit atomic changes
• PR a complete thought or concept

I tend to make changes which causes me to refactor, which makes me go back and change my approach, then I follow it up with adjustment to my refactor. I like to include all of it in a pull request; I don't want to get ahead of myself and have my initial refactor pulled in until my thoughts have been put to use.

I expect the referenced article didn't expect CI to be run on every commit. Thus I conclude that the CI execution is equal, working on a branch or pushing directly to mainline.

Frequent

Take changes from mainline as you work. This creates frequent integration. And since your team is making small changes as well the code is integrated with other development work.

I believe that rebase is the correct way to work on a personal branch. Note that you can have a shared branch. In such situations you don't want history rewritten without coordinating.

Your Team

The primary premise for not needing merge requests is because your team are developers which follow conversations and trusted to know the direction of the codebase.

Congratulations. If there is more than just you on your team, double congratulations.

This is very selfish. The work you are doing has larger implications than avoiding conflicts with other developers work.

These changes should be coordinated with a release of a feature. It should be clearly communicated to the testers that the changes are associated to the feature and part of the release.

If your QA is not interested in changes at this level, that is OK. You will have grouped changes together which can be pulled out when your tester throws you a failure with poor reproduction steps.

I digress, this is less about a PR and discipline with changes. You should setup changes so reviews are easy. Again, check out my series on git.

New Hire

You are now bringing on a new member to your team. They need to go through a review process before they merge. But you don't have a PR culture, sure from time to time members post PRs for larger changes, but it isn't a routine occurrence.

The new hire should not only have review of his code, but should also be doing review of senior developers. Small simple changes are a great place for review.

Conflicts

"Rework" is a be part of Pull Requests. That is at least what the article wants you to believe. I don't know the conflicts the article is referring but there are two types I'm going to cover.

1. Merge conflicts
2. Feature conflicts

Merge Conflict

These occur when people make code changes to the same area of a file. This can be a big source of rework, specifically if refactoring is a core part of the files that you touch.

I am a big fan of continuous refactoring. However these can still come in after a bug fix and receive appropriate communication for review and test coverage.

Feature Conflict

The article suggests that the CI tests will execute and find these conflicts. Gitlab at higher pay tier allows for CI tests to run against a merged to mainline branch, providing those results as part Merge Request.

I also question if the desired method to push into mainline as you work actually reduces these conflicts, over the described discipline here.

Work with your team so people know what is under development and define your features smaller to get them merged quickly. Try to identify conflicts of work planned before it is started.

Conclusion

Do what you want. Every team is different and the items you are tackling aren't the same as mine. I believe that you can get benefit using my suggestions, but it can also be a hindrance when a team goes through the motions and no one is around to utilize its benefits.