As the author of a pull request, it is crucial to provide context since you have a deeper understanding of the changes, while the reviewer does not.
Although not all pull requests require extensive context, it can be beneficial. Sometimes a simple screenshot or short video can go a long way in helping the reviewer understand the changes. And that's especially true when the PR has visual changes, adding a video or a before/after image comparison makes the process much easier for the designer to review it.
We ensure the pull request title is clear and provides a good description of what the pull request accomplishes. Highlight specific areas where we need feedback and include links to related documentation, issues, or other pull requests for additional context.
An example of how a pull request can be structured:
When authoring commits, it's important to be mindful of the context and avoid using commits that don't tell much (e.g. "wip", "update", "new change").
We give preference to using messages that give some context not only to the reviewer but that can be easily revisited and understood in the future by someone else. To help with this, we also use conventional commits to specify the type of change we are applying.
Each pull request should be focused on a small and specific task.
This approach minimizes context switching, making it easier for the reviewer to scan and review the changes.
If multiple changes are necessary for different improvements, we separate them into individual pull requests. This reduces confusion and the risk of introducing bugs or regressions, as the reviewer can concentrate on one thing at a time. Feature flags also help us maintain this separation by allowing us to merge code for large features incrementally without exposing incomplete functionality to users.
In the minority of cases where a PR hasn't been reviewed yet and a new PR builds on that work, we can stack branches to improve the review process and avoid being blocked. In other words, stack branching allows us to build new PRs on top of existing unreviewed PRs to keep development moving
Taking responsibility for the code we approve is essential. If a change causes a bug after deployment, it is partly the reviewer's responsibility.
Code reviews are a core part of our job, not a distraction. When we provide feedback, we don't just point out issues; we explain why an alternative solution might be better. For instance, instead of saying, “We shouldn't use this approach” we suggest, “Here's another way that could be better because of X and Y.”
We have code owners for each repository setup, this way we don't need to request for an specific reviewer in the pull request since it will automatically add the team as the reviewers. In the rare case you need some specific people to see it - you can request them, but it is not the standard.
Understanding the code being changed is vital; if we don't understand it, we can't effectively review it.
To avoid excessive back-and-forth in pull request comments, we usually use Slack DMs or huddle to have better discussions when needed.
After creating the PR, although the PR review is assigned to the code owners of that repository, we share it with the team in the #review-pr
Slack channel so that any of the code owners that are available can review it.
This approach allows for a broader review and helps to ensure that everyone is aware of the changes.
Our CI/CD integration ensures that dependency checks, linting, formatting, building, and testing are automated. Additionally, deploying preview apps can provide better context and facilitate more effective testing.
To track the PR status and keep it in sync with our Linear tasks, we use their automated integration with Github.