Pull requests are a critical collaboration mechanism in modern software development. They allow developers to review each other’s code changes before integrating them into the shared codebase. Following best practices when reviewing pull requests can help teams build higher quality software more efficiently. In this comprehensive guide, we’ll cover proven strategies and tips for reviewing pull requests successfully.
Introduction
Version control systems like GitHub allow developers to isolate their work in branches until it’s ready to merge into the main codebase. Pull requests provide a structured way for developers to propose their completed changes to the team for review.
Before merging a branch into the production-ready main branch, it’s important to have other developers review the changes. This helps catch bugs, improve code quality, and ensure the changes align with the project’s overall architecture and standards.
Thoughtful, efficient reviews of pull requests enable teams to:
- Find defects and mistakes in proposed changes
- Improve code readability and maintainability
- Verify new features work as expected
- Ensure changes adhere to style guides and best practices
- Exchange feedback and ideas between developers
However, reviewing pull requests can also slow down development velocity if not managed well. Lengthy back-and-forth discussions, unclear or non actionable feedback, and careless approvals can hamper productivity.
By following proven practices, reviewers can provide constructive feedback that improves code while keeping up momentum. This guide covers strategies for crafting effective reviews, conducting efficient checks, and reinforcing best practices across your team.
Creating Constructive Feedback
The first step in a great pull request review is providing constructive, considerate, and precise feedback.
Respect Reviewers’ Time
When a developer submits a pull request, their work is blocked until receiving feedback. Leaving pull requests languishing without reviews blocks further progress.
Aim to start reviewing pull requests within two hours of submission. This respects the author’s time and avoids wasting their effort due to context switching later on.
If the pull request can’t be reviewed immediately, communicate this clearly:
@john_doe Unfortunately I don’t have the bandwidth to review this today. I should be able to get to it first thing tomorrow morning. Thanks for your patience!
Provide Actionable Suggestions
Reviews should clearly articulate what needs improving, not just that something is wrong.
Specify the exact files or lines needing change rather than general issues.
Use positive language: “Consider renaming this method to be more clear” rather than demanding blanket changes.
Explain reasoning behind requests: linking to docs or principles is helpful.
For example:
In `utils.js` line 12, let’s use `getQueryParam` instead of `getParam` for consistency with the codebase style per [our style guide](https://example.com/style-guide#naming).
This helps the author understand the context and quickly make fixes.
Keep Critiques Impersonal
Critiquing code is different than criticizing the coder. Keep comments focused on the work, not the person.
- Avoid heated arguments. Take debates offline if needed to prevent blocking progress.
- Consider tradeoffs – there may be valid reasons for some decisions even if you disagree.
- Don’t allow ego to prevent you from accepting others’ ideas.
The goal is collaborating to produce the best end result, not always being right. Disagree respectfully by explaining your reasoning and suggesting alternatives.
Conducting Efficient Reviews
To keep the development process moving, it’s important to review pull requests in an efficient manner.
Understand the Context
Before diving into the code changes, take time to understand the pull request’s purpose and scope.
- Read the description. The author should summarize the goals of the changes.
- Check linked issues or details explaining the background.
- Ask clarifying questions if anything is unclear.
Grasping the context will help you more effectively review the changes.
Run Tests Locally
Don’t just assume the author’s code works – verify it yourself!
After checking out the pull request branch:
- to confirm existing functionality is not broken.
- Manually test any new features to check for bugs.
- Explore edge cases beyond basic happy paths.
Running through these validation steps helps identify any issues before merging.
Check Other Relevant Files
Pull requests shouldn’t only modify core source code files. Look for changes to:
Tests: New features should include unit tests covering the functionality.
Documentation: Docs explaining usage should be updated.
Config files: Build processes and dependencies may need updating.
Changelogs: Release notes should reflect major changes.
Reviewing these other affected files helps catch overlooked details.
Focus on Long-Term Maintainability
Consider not just the immediate changes in a pull request, but how they impact future maintainability:
Will this code be readable in 6 months? Complexity should be justified.
Is it scalable and performant? Shortcuts for expediency now could cause issues later.
Does it follow project conventions? Consistency matters for understanding.
Reject hacky temporary solutions in favor of maintainable code, even if it takes longer.
Reinforcing Best Practices
In addition to providing effective individual reviews, teams should institutionalize processes that reinforce best practices for pull requests.
Use PR Templates
Pull request templates configure placeholder text to remind developers what information to provide each time.
Templates should request:
- Background context linking relevant issues
- Description of changes
- Screenshots of new UI features
- Testing steps and results
- Other specifics important for your team
This standardizes and improves pull request quality.
Enable CI Testing
Continuous integration running automated test suites against pull requests can catch issues early. Use a CI system like Travis CI or Circle CI to:
- Run unit tests for every PR
- Check code linting and formatting
- Verify dependencies and versions
Automated checks like these provide objective measures of quality.
Require Code Owner Approval
Ensure experts review changes relevant to their domains. Using a CODEOWNERS file, you can automatically request reviews from designated owners when PRs touch related files.
For example:
# Frontend team owns JS code
*.js @frontend-team
# Database schema owned by backend
/schema.py @backend-team
This draws on team members’ specialized knowledge.
Limit Temporary Code
Developers may sometimes add temporary code as a shortcut, promising to improve it later. **Treat temporary code with the same rigor as permanent code:**
- Don’t compromise maintainability for expediency
- Set deadlines for resolving technical debt
- Reject PRs that accumulate issues to address “later”
Cutting corners ultimately slows velocity as debt piles up.
Conclusion
Well-executed pull request reviews are vital for maintaining rapid development velocity while producing high quality, maintainable code. By providing constructive feedback, thoroughly vetting changes, and reinforcing good practices across teams, reviewers can dramatically improve collaboration and output.
The best practices covered in this guide, including crafting actionable feedback, efficiently validating functionality, and requiring standards, help developers fully leverage the power of pull requests on GitHub.
Adopting these reviewing strategies allows teams to take advantage of pull requests to exchange ideas, catch defects early, avoid miscommunications, and ensure changes align with the larger codebase architecture and style – all of which enable shipping better software, faster.