Make PRs the BEST PART of Programming
Tags: github, productivity, codereview, programming
Published: 2022-11-29 19:07:54

In this blog post, I'd like to share some of my thoughts on _how_ I've been able to effectively think about software development.

What I'm suggesting is not all that new--most of us do something similar already. I'm not advocating for a revolution. Rather,  I hope to empower teams with a mindset the transforms the rote, painful, and mundane into something powerful and efficient.

Introducing: PRO Programming.

- - - -

In our early days as developers, we often find ourselves searching for information on _what_ to program. "Where is this type error coming from?" "How am I supposed to use this poorly documented library?" Or even, "What does this horrible JavaScript snippet actually do?" But as we mature and grow in our careers, we become more confident in writing code. 

At some point we transition from thinking about _what_ to program and begin asking _how_ we should program. We start to ask more challenging questions, like "How can multiple developers work together on the same task?" "How do I write code changes so as to have the lowest chance of causing a production outage?" "What's the most efficient way of running a software development team?" 

PRO Programming falls into this second category: it helps us know _how_ to program, not _what_ to program.

- - - -
# PRO

P.R.O. (aka Pull Request Oriented) Programming is a way to prioritize developer time and quantize the delivery of code.

The Pull Request is a developer's discrete unit of code.

On a PRO team, each developer has the following 3 priorities:
	1. Top Priority: "Deliver Excellent PRs"
	2. Second Priority: "Unblock teammate PRs"
	3. Third Priority: "Protect the high-quality codebase"

- - - -

Now let's go through and break down how each of these tenants works in practice. First off, what is a pull request?

# The Pull Request is a developer's discrete unit of code.
If you've developed using git in any kind of team environment, you probably understand why everyone pushing to the main branch is a bad idea. Pull Requests (aka Merge Requests or Code Reviews, depending on your stack) are the gatekeeper for the "official"  codebase, protecting against merge conflicts and silly bugs. I've yet to work on a team that doesn't use this model, where automated tests and other team members must approve your changes before they become part of the codebase.

If a developer's job is to deliver a feature, fix a bug, or to some other end modify a technical asset, the way they do it is by making a code change. Under PRO, we no longer think of Pull Requests as a necessary evil, or even just as the process by which I get my code-change in. **The Pull Request _is_ the deliverable.** (Or rather, a series of pull requests is the deliverable.) Importantly, the pull request includes more than just making a code change. 

Pull requests include:
	1. The code change
	2. The documentation
	3. Comprehensive and documented testing steps
	3. Evidence of all automated checks passing
	4. Approval from code-owners
	5. A successful merge and deployment

When I pick up a task on a PRO team, I'm not finished with that task until the Pull Request is complete. As the author, I'm responsible to make sure tests are passing, I've received approval from code-owners, and, even after merging, that the deployment was successful. 

PRs may rely on one another. They must necessarily be the product of multiple developers (approvals). But at the end of the day, they are the fundamental, smallest quanta of code; and through the sum of them the codebase is built.

As a Developer, this distillation of responsibilities into the PR unit allows for the following straightforward priorities:

# Deliver Excellent PRs
To be as effective as possible, a developer's primary area of focus should be the tasks they've been assigned. We don't want to get bogged down in a long review cycle, so we need to make PRs as easy to review as possible. The goal of a PR is to _convince_ other developers that this change will improve the codebase.

Excellent PRs are:
	1. As small and focused as possible
	2. Well tested and documented
	3. Free from bugs

**Small and focused PRs** are easier to develop, test, and review. Don't include any code changes that aren't directly involved in the task you're solving. Only solve one task at a time. Prioritize limiting the number of files and number of lines changed, to make it easier for reviewers. Smaller scope also makes it easier to debug and fix broken tests.

**Well tested and documented PRs** help reviewers now and the whole team in the future. You tested your changes; share how you tested. Explain why the change is happening and include a link to your issue tracker. That makes it easier for reviewers, and 6 months from now you'll be able to remember why you made the change.

**Bug-free PRs** seem like a no-brainer, but hear me out. The code review process is **not** a time for teammates to catch your typos. Before submitting a PR, you should _code-review your own PR_. Asking code-owners to catch simple bugs, syntax errors, and typos in your PR is a waste of their time and, frankly, is disrespectful.

> Tip: When you ask others for PR reviews, you are presenting a finished product that is ready to be merged as soon as they approve. You should be 100% confident in the code _before you ask for reviews_.  

# Unblock teammate PRs
Few great projects are built in isolation... and still fewer projects that pay a good salary. As a team member, I am responsible for doing everything I can to unblock the PRs of my teammates. The most straightforward way of doing this is by submitting code reviews, but it's not the only way. Pair programming, team discussions, and shipping high-quality internal tooling are all important tasks as well. That said, here are a few tips on giving high-quality code reviews:

**High quality code reviews are timely.** When a teammate requests a code review, a code-owner should submit a review within 1 work day. I find it's easiest to have a daily routine of reviewing all open PRs. This applies both for initial reviews and for re-reviews. 

> Tip: It is the PR author's responsibility to request reviews. This can be tedious to request manually on every PR, but [Github allows you to auto-assign reviewers](https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-settings-for-your-team).  

**High-quality code reviews have a certain tone to them.** Writing comments in a way that is respectful, humble, and, as often as possible, open-ended, can lead to more efficient collaboration and faster code changes. Defensiveness and arguing dramatically slow down the PR process.

**Reviews should look at the PR holistically,** but also keep in mind the goal of limiting the scope of each PR. Don't ask for changes to things that aren't part of the original scope--opt instead for opening a tech-debt ticket. Call out changes that are too broad in scope or seem irrelevant. More small PRs are better than fewer big PRs.

> Tip: If a developer repeatedly receives the same feedback on their PRs, they should discuss offline with senior team members how they can prioritize delivering excellent PRs.  

- - - -
# Protect the high-quality codebase
The metaphor of PRs as a gatekeeper for the codebase was used earlier in this post, and it still stands. Each and every PR could be a trojan horse for debilitating technical debt. There's a balance to draw between delivering code quickly, unblocking teammates, and enforcing the high standards we have for our codebase.

> Tip: PR reviews around code quality, style, and functionality can be one of the most effective ways of sharing knowledge within the team. If you don't understand something, post a comment asking a question. If the PR author made some mistake, help them understand your team's best practices.  

**Be a stickler.** Read every line of code in a PR you approve. If you approve a PR with tech debt or a bug in it, you are just as much responsible as the author if the PR -- if not more.

**Validate the functionality.** A comprehensive test suite can confirm a lot of things, but it often can't confirm that a PR does what it says it will do. Manually validating the changes in a test or local environment is the best way to confirm the quality of a PR.

**Automate the things humans are bad at**: formatting, import sorting, unit testing, etc. [IDE extensions](https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode), [git hooks](https://typicode.github.io/husky/#/), and [automated workflows](https://docs.github.com/en/actions/examples/using-scripts-to-test-your-code-on-a-runner) are highly effective at this sort of task. You shouldn't ever need to post a PR comment asking someone to fix indentation or import order -- use tooling. Whether you use Github Actions, Circle CI, Travis, or something else, automated tests are vital for maintaining code quality.

- - - -

The principles and best practices of PRO Programming have empowered me and the teams I'm part of to write code quickly and effectively. I hope they give you clarity and level up your teams too.

Kaeden Wile
[https://wile.xyz](https://wile.xyz)