I don't think pull request etiquette is something widely talked about and reviewing is a skill that eluded me for years of my career. This post is as much the internet wanderer who stumbles upon it as much as it is for my future self. These are guidelines I aim to follow

Before opening

How big is the change?

If you have a feature or architectural change that is going to require "heavy lifting", these are the sort of things that should be worked out pre-pr. It's not a pleasant process for either party (reviewer or reviewee) when a pull request needs to be closed.

Does it meet the feature request / spec?

Sometimes, I can find myself so eager to push something out so I can have that feeling of being "done." Reaching the finish line is rewarding, but it's worth nothing if you forgot the baton mid-race. And by this I mean, have you thoroughly went through each requirement and request and considered their edge cases? It seems so obvious now, but I can remember how as a CS student or junior engineer that I wrote off the details. But, this 'mostly' works! I might have thought. There are some areas of development where being pedantic pays dividends, and accuracy of an implementation will save both you and the people around you time. It has the added effect of showing people around you that you care about your work.

Know where to merge

If you're working on an open source project or joining a new team, you'll want to figure out how the project is using branches. For example, many web applications may use a development, staging, master branching scheme where each branch corresponds to a different application environment. A CLI application may use a totally different scheme.

Opening a pull request

Verbose pull request info

Have you ever looked at a pull request and it was blank? No info, no related issues. Typically, in every pull request I'll include both a Summary and How to Test. Below is a basic example of adding rate limiting to an application's login page.

Getting a reviewer

Sometimes, whether working on open source or just within your team at work, getting someone to look at your code can take time. If I'm working with a piece of code that the original author is the only person with extensive knowledge, I might request a review from them or at least ask them to look over it.

Since code review can a real time investment, I've found using a bot that selects reviewers in a round-robin fashion to be the most dipolomatic for teams larger than three people.

Reviewing a pull request

1. Does it pass the basics? (linting, code formatting, style guide)

Having a linter (golint, eslint, rubocop, etc.) run when a pull request is submitted can save a team time and make sure code is, generally, following all the same rules. Not having to focus on syntax or general code smells caught by linters can allow reviewers to actually focus on review.

2. Does it actually work?

I've seen many PR's merge and then reach a staging or production enviornment to only find the feature actually doesn't work. Actually checking a pull request out and testing that if satisfies what it intends to do is an important.

git fetch origin pull/ID/head:BRANCHNAME

git checkout BRANCHNAME

source

3. Does it meet all cases in the spec?

This feels like a gray area, because the actual details of the implementation should be between the requester and the person opening the pull request — but understanding changes helps to build context and no programmer is perfect.

Other antecedtoes

Be constructive and assume people are trying their best

I don't think anyone wakes up and says, "I'm going to write the worst code possible today. I can't wait for someone to try to review it!" The code you're reviewing is a culmination of this person's time, so, in some ways, you're looking at how someone has chosen to spend their time.

Provide timely reviews and replies

If review drags on for days or weeks, PR's can lose momentum and the author can move on to something else and forget the specifics of their implementation. Each time the author has to come back and refresh their memory, that has a cost.

Consider doing a PR demo for the reviewer

I've done this for the past couple of years when something is complex enough that it warrants a walk-through. Details like design decisions and architeture are sometimes best suited for a real-time conversation.

---