Introduction

This document describes how we use Pull Requests at Township, and what comprises a well-formed PR.

Pull Requests at Township

The purpose of a PR is to provide a summary/preview of the work that has been done to those doing code review for a feature, bug fix, code change, etc.

A PR should consist of as many of the following as needed:

Title

Start with a feat, fix or chore tag (conventional commit style)
Match the Linear ticket title somewhat, if possible
Tag drafts with WIP so its apparent the PR is not yet ready to be reviewed.

Description

Summary of the reason for the change/new feature/bug
Summary of the approach taken, reasoning behind engineering decisions
Summary of what your change/feat/fix accomplishes

Loom video/Screen Recording

Demo of the changes, walkthrough, setup, etc.
Try to keep this under a couple mins or less.
Use a screen recording if no verbal explanation is needed and it is something quick.

Screenshots

Highlight before/after of a UI addition/fix/update etc.
Add label to screenshot

List of files added

Ex: adding a bunch of icons may look like a lot of code has been added when in reality it’s just a lot of new files. It could be helpful to list those in the PR description so the code reviewer knows those may not need to be looked at in detail

Links

to Figma if its a UI ticket
to Linear - especially if the project is not synced with Linear
to newly added dependencies
to reference articles used

Pull Request Process at Township

Submitting PRs

When you push your first commit from a branch, Github will display a banner with a button to open a new pull request. You then need to fill in the PR description and submit it as a Draft PR or Ready for Review PR.

Draft PRs

start a draft when you’ve pushed up some code but aren’t ready for final review
start writing the full PR description, update as progress is made/commits are added
maybe include to-do items
include blockers/what you’re having trouble with & need help on
link to the PR in the project slack channel to draw attention to it if help is needed

Ready for Review PRs

Change Linear ticket status to “Ready for Review”
Cleanup commits (Rebase/squash)
Make sure your branch is pointing to the right parent branch, and is based on its most recent commit
Convert an existing draft to Ready for Review
Make sure CI has passed
Tag reviewers
Tag yourself as assignee
Wait for feedback & comments
Address requested changes

Addressing Requested Changes

When in doubt, re-request feedback after a code reviewer has requested changes on your PRs.

Nits/Cleanup

These are nice-to-haves, but may not be required. The code reviewer will usually specify this (ex: “maybe make these small changes then it’s good to merge”).

Reviewer likely doesn’t need to re-review after these changes.

Fixing indentation
Better function naming
Adding/cleaning up comments
Removing left-in debug statements

Refactoring

These changes should get reviewed again after fixing. What you’ve done works, but could maybe be written more concisely.

Re-request review after addressing feedback, unless the reviewer specifies otherwise.

Cleaning up repeated code
Maybe changing an approach/function design

Required Changes

These changes must be made before the PR can be merged, and will possibly get multiple rounds of CR.

Always re-request review after addressing feedback.

Found bugs
Missing functionality
New/updated requirements for the ticket that now require additional code change

Merging PRs

Requirements for merging:

2 approved reviews - typically with an explicit “looks good to merge” (LGTM) note

Possibly only 1 required - with approval from ? Scott? ex: very few people on a project, maybe can’t realistically get 2 reviewers in time

No merge conflicts!

Performing the merge:

Hit that Squash & Merge button
Update ticket status in Linear to In Review (if not automatically done by Github)

Additional Reading

Pull Request & Review Guide

Library

Github Docs - About Pull Requests

Example PR#1

This is a new feature PR. There were a lot of files added/changed so aside from the description, loom video, figma links and screenshots, files added/files changed section was added to make it easier for the review. Some of the files were simply icon files so they didn't need to be reviewed as thoroughly. Sometimes you have to touch a lot of making reviewing your PR time consuming. This PR also had a conversation between the reviewer and the assignee regarding suggested changes to the PR.

Example PR#2

A bug fix PR that describes the problem in detail, explains the desired behavior, and summarizes how these changes apply a fix.