The Importance of PRs
Software development is not about writing code. It’s about structure.
Sure, the code expresses that structure, and that’s the system we’ve stayed with for many decades because text-handling is easier than writing a GUI. The first programming languages were about assigning values to memory locations, and since memory was viewed as a series of blocks, this was intuitive. Software complexity has ballooned and the modern program is about more business-intuitive concepts broken down into classes and functions.
Stay with me. I’m going somewhere with this.
As a front-end developer, one of the biggest mistakes I ever made was to ignore a message. The message was sent by the front-end tech lead, who was in charge of handling pull requests (PRs), posted in the front-end Skype group. He said “please let me know if you’re going to refactor any code”. I thought to myself “eh, he’ll see what’s changed when he gets to review the PR”. This was a huge mistake, and I’ve regretted it every day since. I should have brought it up with him at the time, and specifically said to him that this is what PRs are for. The sub-text of his message indicated he didn’t understand how refactoring should work as part of an efficient software development process, and I should have addressed that immediately.
Myself and the FE contractors ended up pulling in different directions. Deviant file-system patterns, non-specific naming conventions, no attempt at linting and no TDD were contributing to disorder and, by extension, increasingly difficult bugs. The architecture was suffering. All because I didn’t have that discussion with a lead who clearly wasn’t using PRs to read the changes in terms of the existing structure of the code.
Of course, for this to work, developers need to write short PRs so that they can be read. Need to work on something which requires the feature you just submitted a PR for? Branch from it to make the next feature, then you can PR that. Someone else’s PR been merged? Merge the production branch into yours. Conflicts? Good opportunity for a discussion, unless you can fix it without help (and most of the time you probably can).
I can at least say I’ve learned my lesson. When the tech lead was replaced by somebody who did use PRs to check for structure, the difference was stark. Somebody who would leave comments in my PRs elaborating on the message of “please follow pattern X” helped me deliver better quality code than I would have without them. I could see the difference for myself and it drove home for me why, specifically, we have PRs. There are machines to handle linting, and looking at functional changes is best done by running the actual code on your machine if you want to do that, and a good build process runs the automated tests, but the actual PR is for looking at the code and reading the structural changes made or for any new pattern precedents set. It’s to get an idea of the shape of the changes.
This isn’t just about one developer’s PR. If all the PRs in a team go through a senior member, all the PRs can be checked for following the same patterns. Developers will not pull in different directions and create chaos for each other if they’re being shown where to follow the same patterns. The code will be cleaner, maintenance will be easier and the overall project will take less time to deliver while developers are not as likely to struggle with finding deep bugs in the code.
PRs are for managing the software structure, and good structure allows for good code quality, and good quality code is a good quality product.