Pull requests play an important role in any large software development project. They facilitate efficient code review, reduce bugs, track progress, and help coordinate a shared understanding of large code bases. Some type of pull request mechanism is built into every version control system and they can be integrated into a wide variety of notification systems including email, chat, issue trackers, and project management systems. However, there is not a lot of information on best practices for making pull requests work smoothly and efficiently as part of different styles of application development workflows.
I recently caught up with Rod Johnson to learn more about best practices for getting the most out of Git and GitHub pull request workflows. Johnson created the Spring Framework, and more recently Atomist, a framework for software delivery infrastructure.
What are some of the different ways you use pull requests in your app dev workflow? i.e. what tools do you integrate to consume pull requests and facilitate better communications among your team?
We use the GitHub Review mechanism, with Atomist to facilitate communications via GitHub and Slack. You’ve forced me into a product plug here, but since we started using Atomist ourselves, we haven’t looked back!
When we raise a PR, we typically choose reviewers using the GitHub UI. We have Atomist running on our GitHub org and community Slack team, so reviewers will get direct messages in Slack to let them know. We all tend to live in Slack, so that works a lot better for us than email alerts. Atomist attaches labels to the PR. Of course, you can do that with other tools, too. I think it’s a good practice for automations to add information that helps speed review.
Atomist is based around handling events, and a PR is an important event that we can add custom code to handle at team level rather than individual repo level. This enables consistent policy.
In our teams, Atomist auto-merges PRs when reviews are complete and automatically updates the change log on merged PRs. It automatically runs code quality checks and linting on all pushes, even before a PR is raised.
I don’t think integrating a host of tools into your software configuration management makes sense. It’s better to plug in one portable thing that gives you a rich model for handling events. The same approach works on GitHub, BitBucket and GitLab. Instead of managing a number of integrations, you just integrate Atomist and use its programming model. For example, our “autofixes” are doing more and more things over time and saving us a ton of work. Initially we just did linting. Now we add missing license headers, format imports etc. And it’s easy to add further things like this without changing any GitHub setup. Our customers use this to integrate tools like SonarQube.
You can see the output of this at https://github.com/atomist/sdm and in our community Slack (join.atomist.com).
Do you find that one style of pull request workflow works best across a single organization or are different workflows better for different types of development?
We use PRs and try to keep branches short-lived. Any branch that lives longer than a day or two is questionable IMO. The default branch should always be fairly up to date.
What would you consider to be best practices for improving the use of pull requests to speed development, improve quality, and keep teams on the same page?
I think it’s good to review PRs. It can help with quality, but it also ensures that knowledge is clearly communicated in the team and makes it easy for people outside to follow. In open source, the latter is important as you’d love to inspire and empower people to contribute. I think tone is important in reviewing PRs. I don’t believe in accepting all PRs, but it’s crucial to be clear and respectful in comments. The PR mechanism can be an amazing way of working toward a better solution than any individual envisaged when setting out.
What kind of challenges have you encountered in making pull requests work smoothly?
Every challenge we’ve had involved long-lasting PRs, where you get into rebasing hell, where you need to continually rebase a long-lived branch from master, potentially resolving conflicts
What do you think of useful ways of thinking about pull requests as more than just a communications feature baked in to Git, GitHub, etc. and as part of streamlining the app dev to production lifecycle?
I like it when the default branch is deployed to production and you can use PRs to drive promotion. Of course, that isn’t appropriate for all organizations–some need a separate formal promotion mechanism–but it’s great if it is possible.