# Code Reviews Here are some best practices for quality, inter-squad code reviews. One downside of breaking up OKR work into 2-3 person squads is few teams have more than one backend or frontend engineer, and of necessity, most code reviews will come from outside the squad. ## Authoring Pull Requests Include all the context necessary so someone unfamiliar with the change can effectively review it. A one-sentence pull request is unlikely to contain enough detail so someone outside the squad stand-ups can effectively review. - Essential to include - Background - Reasoning for why the change is being made - Test setup steps in staging or hotdog so the reviewer can reproduce any manual testing (automated tests are obviously better). - Considerations, caveats, alternative choices, and why they were not taken if applicable. - Nice-to-have - Put Jira ticket in the pull request title (e.g. _PROF-6666 foo_). This will automatically create a link from the ticket to the pull request. - Put a Jira ticket ID in the pull request body - `[PROF-6666]`. The Jira bot will automatically populate the full link later. If the ticket is well-formed, it will also include context helpful to the reviewer. ## Reviewing Pull Requests Reviewers and authors experience an awkward power imbalance with the scales tipped to the reviewer. Even though the author has more time invested, they are dependent on the code reviewer’s oversight. Here are some tips to make reviews more collaborative and higher quality. ### Synchronous Reviews - Pairing is the highest bandwidth possible for code reviews. Both author and reviewer are on equal footing and are putting an equivalent effort into the change set. This is often not possible with small squads and a key downside is the high-bandwidth knowledge transfer is ephemeral. - Review 1:1 - scheduling 30 minutes to go over a review is similarly high-bandwidth and a great tool for complex changesets; similar downsides as pairing ### Asynchronous Reviews If the review can not be done synchronously, there are still things the reviewer can do to level up collaboration and reduce the power imbalance. All of these tips move the scale on time invested and power balance between author and reviewer. - Pull down the feature branch locally rather than read it on Github. - Run the tests. #### Create a pull request against the pull Rather than dictate suggestions as comments, try out the changes you want in another branch. This demonstrates respect for the author’s time. If the review feedback is a good idea, it’s easy for the author to merge or cherry-pick the suggested changes. A low-effort review comment that forces the author to spend even more time can be demoralizing and should be avoided if possible. Demonstrating the change rather than dictating is often net-time efficient and collaborative. This technique is an effective shallow-feedback filter. Often in trying out a suggestion, the reviewer realizes a 5-second burst of insight during the review lacks context and is sub-optimal. Not adding that bit to the review is also a net-time improvement. The author is (hopefully) receiving constructive feedback, but a reviewer can struggle to frame feedback as either a “nit” or go nuclear and block the review with “Request changes.” The above tips can help with the framing and make the experience better for both parties. ### Review styles to avoid - _the slow drip_ - Author incorporates feedback as the reviewer slow-drips multiple rounds of “Request changes” - _the low effort opinion_ - Software engineering can be an artistic act and everyone has style preferences. It’s a better experience for all parties to have a nits discussion 1:1 than in the durable record of code review comments. - _the rubber stamp -_ arguably worse than no review ### Tooling tips Github’s pull request UI is still `:vomitemoji:`. IntelliJ IDEA has [tooling for Github Pull Requests and reviews](https://www.jetbrains.com/help/idea/work-with-github-pull-requests.html "https://www.jetbrains.com/help/idea/work-with-github-pull-requests.html"). Advantages include - code navigation - shortcuts for navigating the file diffs - IDEA's diff tooling ### More resources Chelsea Troy wrote a series on pull requests. - [how to write a good pull request](https://chelseatroy.com/2019/12/13/async-collaboration-1-submitting-pull-requests/) - [how to create a good pull request review](https://chelseatroy.com/2019/12/18/reviewing-pull-requests/) - [how to write commit messages](https://chelseatroy.com/2020/01/08/storing-context-in-commit-messages/)