3 ms·
Mostly I think this is a mix of (1) "good advice for dealing with junior devs" and (2) "good advice for dealing with people new to a technology." For #1 don't
by repsilat 9y ago
Mostly I think this is a mix of (1) "good advice for dealing with junior devs" and (2) "good advice for dealing with people new to a technology."
For #1 don't hurt their feelings, for #2 teach them instead of expecting them to know.
Going through the points in the article with these lenses:
"This component should be stateless" is reasonable feedback to give to someone familiar with React. Maybe they had a good reason for not making it stateless, maybe they made a mistake, but giving an impromptu lesson and providing references probably isn't appropriate -- it's condescending. Again, fine for someone new to the technology. Audience is important.
The "Avalanche of Comments"... As someone who doesn't really do it, and as someone who has been on the receiving end of it, I always appreciated the thoroughness. It's not judgmental, it's a checklist, and it's helpful.
"Spew emojis" hit this taxonomy too. If my code is ugly enough to warrant one (and it sometimes is), I want to know it has crossed a line, and someone telling me that I am a terrible person who has ruined their day (or given them cancer) is a perfectly reasonable, light-hearted way to do so. Constructive feedback is useful too, but for a senior dev they probably knew it was ugly and just need telling that it was too ugly and that they should try harder. Obviously not appropriate feedback for junior devs, though.
My disagreement with #3 doesn't fit that taxonomy though, mostly I just hate process. Most refactors are super fast, fairly mechanical jobs. Both you and the reviewer have that code in your cache, and it will likely never be a priority again. Take some initiative and rewrite the thing, it'll only take 20-30 minutes. Making a ticket increases debt instead of paying it down.