By the time I’m writing this post, I will have been at my first engineering job for roughly 1.5 years. During this time, I’ve learned and experienced a lot. But the moment that stuck to me the most was around the time when I was finally starting to feel comfortable around my colleagues and about my technical capabilities. I had two features, which were related to each other but required completely different feedback, mixed in one branch and thought it was too much time-consuming to split it up. So I didn’t. Shortly after opening the Pull Request (PR) for it, one of my colleagues left a review stating that really hit me hard. The feedback was that by no means should reviewing be made harder for the sole benefit of making the code proposing easier. Although I know my colleague had no ill intent, I felt like I was instantly put in my place, but in a good way. His comment was 100% correct, and I was mostly disappointed in myself for forgetting about such a relatively trivial thing.
This experience sparked me to re-think how I approach the reviewing process from a code proposer’s perspective, how to structure my work, what my duties are in that role, as well as how to actually fulfill those duties. The rest of this post will be a write-up of this self reflection and what I learned from it, starting with work structure.
Most likely, you have or will come across the advice to keep your changes and pull requests as small as possible. The benefits are elaborated on extensively throughout the field: being easier to review, to merge, and to roll back if necessary are to name a few. While more often than not this is true, I think that it isn’t the best approach for all scenarios, as with everything in software engineering. At least in the sense that it shouldn’t become your primary goal to be make your work as small as possible as that can also be counterproductive for both your colleagues and yourself. But how should you structure your work then?
Well, that depends entirely on your team, the people who will review your work. The best pull requests from a reviewer’s perspective are the ones which are easy to understand the context of, to go through the code, and to reason about the approach related to its concrete result for the users.
In the past, like when I was actively contributing to open source projects like Storybook, it made the most sense to create one pull request per feature. The available time and effort from others were limited, so splitting it up into multiple smaller pull requests would cause too much overhead for the reviewers, while increasing it beyond a single feature would make it too complicated to review and maintain properly over time if there’s a period of decreased activity.
At my current job, however, we’re with a relatively small team and have the infrastructure and culture set up to be able to iterate our product at a relatively high pace. The possibilities of being able to quickly iterate is also something that we make a focus of in our team. Considering these circumstances, it makes more sense for me to break down individual features even further and create multiple small PRs rather than a single big one. This is the team’s work flow and what everyone finds most comfortable, so it’s my job as a code proposer and colleague to fit into this style.
This is for me the main thing to keep in mind: as the one proposing changes, you should constantly and actively be thinking about how you can make the reviewing process as smooth as possible for your reviewers. The key aspect to this is not necessarily to keep your proposals as small as possible, but to understand your reviewing audience and how they prefer to review.
This is not a straightforward thing to understand, especially if you’re new to a team. But this will come with time as you will have collaborated more with your colleagues and learned their preferences. While I don’t have a step by step guide that will guarantee success, I can talk about you a few things that I noticed were effective and appreciated by my colleagues:
- Imagine it from their perspective. Ask yourself, what is the main focus of your changes and what do you want your reviewers to focus on? Provided the code and the issue it’s trying to solve, would you be able to review it properly? What else would help your reviewers in understanding your work more quickly.
- Make it easier for your reviewers to go through your work. After asking yourself the previous questions, there might be a few things that you have already thought of that would assist your reviewers. In general, you as the proposer should provide some guidance or resources to make reviewing as easy as possible. Small things that I have noticed are always very effective and appreciated in my frontend work are screenshots or recommending the reviewers to ignore whitespace changes.
- Be committed to following up on feedback. Nothing feels worse than not gaining a response after committing to a review, which goes both ways. Your colleagues have spent valuable time into reviewing your work. The worse feeling for them would be that nothing is done with their review and that they have wasted their time. Make sure you actively communicate back to them that you have at least received their feedback. Even better if you follow up afterwards that you have processed it. In Github you can resolve comments, which in my experience is one of the clearest way of communicating back to your colleagues. Even a simple 👍 emoji will do the trick!
Reviewing is a core process in software development. It is a form of communication between two different parties. More often than not, we are caught up in our own perspective as the code proposer and forget about the person on the other side who has to review your code. Optimizing their end of the workflow is arguably also part of your job as a colleague. There is no clear-cut solution on how you should do this as it is different for every team and individual. Rather, you will get there eventually by thinking. You need to keep thinking about your work and about one of the most important audience for your code: reviewers.