As a Software Developer working on a product squad, one of my daily responsibilities is to review other developers code. But, when this code change is too big, to review it can be a torture. Usually, you don't know where to begin and it takes too much time to understand the impact of the changes that your colleague is doing.
Here are some tips that I use to make my Pull Requests more "reviewable".
It makes no sense to talk about splitting a Pull Request if it is small and understandable. But, when the Pull Request have more than 200 lines of changed code, you can start thinking about splitting it.
Another metric that I follow is the number of files affected by the change. If there are more than 5 changed files, it could be a sign that you should split your change into small pieces.
These 200 lines of code can't include tests because tests can be as extensive as necessary, and it is ok. Tests usually have a lot of lines just to setup the test environment.
describe MyClass do describe '#method' do describe 'when you have to write 8 lines of code just to make your test understandable' it 'considerably raises the Pull Request changed lines of code' ... end end end end
To classify a Pull Request as extensive, you have to consider only the changes that you made on the code that will run.
Any Pull Request must have a goal. Sometimes it can have multiples goals, and that is where the problem is.
Last week I had to work on a project that I never touched before. When I started working, I realized that the project was not ready to run on a dockerized environment. So, I had to create some docker related files (Dockerfile, docker-compose.yml, ...). It was nothing related to my initial goal, so I opened a Pull Request only to setup the docker environment. In this case, the goal is simple to understand for anyone who will be reviewing.
Another example is to setup or change linter rules. Sometimes there is a rule that never affected you, but in that specific change, it started affecting and you have to deal with it. But, to deal with it can mean that you will have to change a bunch of code in random files that are not related to the initial goal of your Pull Request. It is a smart move to open another Pull Request for these linter changes. Also, this example is perfect for the next tip.
When an extensive Pull Request have changes on files that are not directly related to your initial goal, it is time to ask yourself if it could be another Pull Request.
Usually this happens when you have to refactor one piece of code that affects multiple unrelated files. You could just do the refactor first and then apply the necessary change to achieve your goal. Again, here you transform 2 goals into 2 Pull Requests.
To refactor is the usual case of change that does not affect any current behavior. Sometimes you just find an opportunity to optimize a query or to replace some code that is not in the best place. I am not going to tell you that "You must not change working code" because I believe that every developer wants to change working code when they realize an opportunity to make that code better.
If an opportunity appears, you can:
- Change the code if it is a small change;
- Put the change on your to-do list and work on it right after your current work;
- Create a Technical Debt to work on it later if you don't have time to spend on this type of work.
Tip #4: Sometimes you just realize that you can split that big change into small pieces later, and it is ok.
I usually start working without thinking if I could split the major task goal into small achievable goals. Then, when I achieve the major goal, I realize that it could be splitted.
Then, I list all small goals I think that makes sense, open a fresh new branch from the main branch and start working on it. After finishing the first small goal, I open another branch, now from the branch that I was working on, and develop to achieve the second small goal.
After finishing all small goals, it is important to tell reviewers that every small goal is a part of a bigger goal, especially if they have an order to be respected.
One of the most satisfying things to do at work is to review a code that you don't have any problem to point out and you understand every single change that has been done. If you create this experience for your colleagues, I am sure that you will be influencing them to also do code changes in small pieces.
Nobody likes to waste time on a Code Review. Don't be that developer.
Git branches photo by Microsoft Docs
Thanks to Jéssica Veng for reviewing the text.