DEV Community

Cover image for Make PRs the BEST PART of Programming
Kaeden Wile for Developers @ Asurion

Posted on

Make PRs the BEST PART of Programming

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.

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, git hooks, and automated workflows 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

Oldest comments (0)