Some times it comes in the shape of a notification, but many other as a harmless direct Slack message:
- Hey Dani! Good morning.
- Hey, good morning, how are you doing?
- All good, and you?
- All good too, thanks.
- I’d like to ask you if you could review a PR I just created…
- Sure! No problem. As soon as I have time I’ll take a look…
How naïve… Time?
You go to the PR and find 72 changed files, with 1298 added lines of code and 872 deleted!
Time? What I need is to take some days off to review your PR!!
Not so long ago I received another one that had… wait for it… 255 changed files!! I don’t remember how many lines but… 255 files!!
And the last one… It came accompanied with a very concise sentence: “Please, help review this PR”, and the link. Easy-peasy. I open the PR and …… 🥁 🥁 🥁…… 5825 new lines!!!
I’d need to borrow a whole new life to review that!
Although there are some developers who are against having Pull Requests I still think we can get benefits from them… I’m still of the opinion that PR are a useful tool we can get benefit from it… Of course, when we use it properly.
If you send me a PR as long as the British encyclopedia. If Don Quixote is a fairy tale compared to the PR, then probably it’s not that useful.
I understand that sometimes it will be unavoidable. You rename a class, a package, method or extracts part of a class to a new one or bumps a library to a new version that creates a breaking change… I understand this kind of changes might generate tenths of changes in the project. Changes that might affect many many files and lines.
But that’s not the problem. Because this might usually generate easy changes easy to review.
The problem is when you create a PR to review that you have been working on them since you were a child. And “since I’m working on this, I’m going to refactor that” or “This other card is related to this one, I’m going to work on both in the same PR”, or…
Do you know what happens when one needs to review such a long PR?
One is not completely focused on the PR and loses the track of it very easily. A colleague comes by to ask you something and distracts you and when you come back to the PR you don’t know where you were looking at. If you want to understand all the changes and the context you must jump among hundreds of lines to follow the thread of the code. Then another colleague comes by and ask you if you want to take a coffee. You go because you need to take a breath but when you go back to you lovely PR you are completely lost. Then you go home (because you have a life beyond this PR) but the next day you lost the track. You get married, you have kids… until one day you get tired of that PR that you are losing the track continuously and think: “You no what? Fuck it, I’m going to approve it and see what happens. If something goes wrong we’ll fix it later on in another PR. I don’t care”
If we want that a PR is effective and adds value to the development process it must be not so long. It must allow the reviewers to understand it, review it without much extra effort and easy to follow the new code. If we create a monster and we use one PR because “Since I’m with this I’m going to do that refactor”, “I’m going to implement this task it was still left”, “I’m going to include my Degree Final Project”, it loses the value of a PR.
Probably you are doing more job that you are supposed. Probably the task or story is not well broken down.
Because when I have one of those cases in front of me I start to consider that probably trunk-based development philosophy is not a bad option.