Better Pull Requests

Our code eventually has to work with other people’s code. While the way we used to do this was much more chaotic (just checking in to the main branch and hoping you didn’t screw up). This might have been acceptable in the old days with once-yearly deployments, but it simply will not withstand an environment with more frequent deployments. Additionally, over the years we’ve learned a lot about how to make cleaner, more maintainable code that can be more quickly understood by other developers. However, in order to do this well, one of the best things you can do is to make sure that code meets a standard of quality before it is committed into the codebase. While you could just write a coding standard and try to enforce it, that sort of top down approach is suboptimal. Instead, you’ll find that things work better when the entire team agrees on and protects the coding standards. PRs (or Pull Requests) are a great way to do this. Essentially, what you do is work on a branch until you have a logical unit of work done. Then you push your branch up and start a pull request into another branch (either your feature branch or master). When the pull request is ready, one or more of your teammates will review the code and make comments on it (or approve it if it is “perfect). If there are comments, you respond to them, either by explaining why you did something a certain way or by fixing your code, and pushing back up. Since your code stays on the same branch, pushing updates should automatically update the PR so that the rest of the team can see the updates. Once the PR is approved, it should be merged into the target branch. PRs can be nerve wracking, especially if you are early in your career or working with an unfamiliar team. However, it’s absolutely necessary if you want to have a clean, maintainable codebase. Therefore, you need to follow some simple rules to make it more likely that people not only review your code, but that the process will be as painless as possible. While making a proper PR has a lot of technical considerations, it’s also very much a social issue. And like most social issues, there is a lot to be said for considering how other people will react and feel when you do something. As a result, you’ll be far better off with a consistent PR process that is built around making things as easy as possible for whomever is reviewing your code. Episode Breakdown Keep PRs small. If you want PRs to turn around quickly, limiting the amount of stuff you change is a great way to do that. People are more likely to put off reviewing larger PRs. Remember that the difficulty of evaluating code scales at least quadratically with the number of separate things changed. It quickly gets unmanageable when you change a large number of files in a single PR. Smaller PRs also help with testing scope, especially if your QA writes tests on their own. Smaller PRs make it easier for them to reason about what tests they should write. Small PRs also mean that another developer who runs into merge conflicts with your code will have fewer issues to sort out. Pre-review your code before asking others to review it. You should look over your code to make sure that you’ve committed everything you think you have. It’s easy to accidentally forget to add files to source control. Additionally, this is also a good review of the changes right before you (potentially) engage in conversations about the code. This is especially helpful if you’ve been working on your code in between meetings and other interruptions. It helps refresh your memory. This is also a good time to check for things like spelling errors, code standards violations, and the like. While you might check before you push an update, something about looking at the code in a different environment (for instance,