It is a polite comment, almost apologetic, on your pull request. Sorry if this is a silly question, but I found this function quite hard to follow. Could it be split up? It comes from the engineer who joined six months ago. You wrote that function in an hour, you know exactly what it does and your first reaction is a small flare of irritation.
Notice that flare, and then put it down. Feedback that travels upwards, from a junior to a senior, is rare and expensive. It took courage to write that comment. How you respond will decide whether they ever write another one, and whether anyone else on the team does.
Start by assuming they are right until you have checked. You are the worst possible judge of whether your own code is readable, because you already know what it means. They are the closest thing you have to the person who will open this file in two years with no context. Their confusion is a measurement, not an opinion.
So read the function again as if you were them. Where would you get lost? Is there a variable name that only makes sense if you were in last week's meeting? A clever line that saves three lines at the cost of a minute of thought? Usually you will find something within a couple of minutes.
Reply properly. Thank them, specifically. Good catch, the second half was doing two jobs. Then change it and tell them what you changed. If you disagree after a real look, explain why in a sentence, kindly, and leave room for them to push back. Never answer with it is obvious, even if you are tempted.
If there is a pattern, say so in public. Mention in the team chat that a review comment made your code better. It shows the newer people that questioning a senior's work is welcome here, not risky.
And ask for more of it. Next time you open a pull request, tag them deliberately and ask, does this make sense to someone seeing it for the first time? You will write better code, and they will learn that their view counts.
– Asael Shinder
Top comments (0)