The pull request
touched eleven files.
It renamed a class
that everyone agreed was badly named.
It reworded some log messages.
It moved four methods
into a new service.
The review lasted two days
and drew thirty one comments.
Should it be Manager or Service.
Should the log say failed
or could not.
Is helper a word we allow.
Every comment was polite,
and most of them were right.
It was approved on the Thursday.
On the Monday,
a support agent
opened an admin page
she had never been able to open.
One of the four methods
had carried a role check
on its first line.
It did not survive the move.
The diff showed it plainly.
One missing line,
inside a long block of red,
because the whole method
had been deleted in one place
and pasted in another.
Nobody commented on it.
Not one of thirty one.
This has a name.
The law of triviality.
Parkinson described a committee
that approved a nuclear reactor in minutes
and argued for an hour
about the bicycle shed.
Not because they were careless.
Because everyone understands a bike shed,
so everyone has something to say,
and saying something
feels like reviewing.
A reactor asks you
to admit what you do not know.
A name only asks for an opinion.
So the discussion grows
as the risk shrinks,
and the number of comments
starts to feel like safety.
It is the opposite.
Thirty one comments
told everyone
the change had been looked at hard.
It had.
At the wrong part.
So change the order you read in.
Before the first comment,
ask one question
of every diff.
What in here could hurt someone.
Find that file.
Read it first,
and read it slowly.
Ask the author to name it
in the description,
in one line.
Moves and renames
go in their own pull request,
so a moved method
shows up as moved,
and a deleted line
stands on its own.
Save the names for last.
They will still be there.
The number of comments
measures how easy the change was to talk about.
Not how carefully it was checked.
– Serguey Asael Shinder
Top comments (0)