3 ms·

In over a decade of enforced code review experience, I've had one developer who was too immature to take feedback. Some folks take it personally and they should

In over a decade of enforced code review experience, I've had one developer who was too immature to take feedback. Some folks take it personally and they shouldn't as long as the feedback is about the code. This requires some work on both reviewer and reviewee. The guy who couldn't take feedback (person A) had code merged in that wasn't properly tested. Person B said, "hey A. I could use some help. We wrote some tests around $feature that were missing and the tests show the feature doesn't work. We see $unexpected results. Wanna make sure we agree on how this should work." Person A: no, it works, I tested it. B: could you help us identify the flaw in the tests then - they seem rock solid. A: no, my code works. B: ... ok, can you join us and talk through it? A: no, it works. A was removed from the team after management came in and A continued to not acknowledge his code could be wrong. This was aberrational. We, as an org and as a team, constantly strove to keep the focus on the quality of the code. And, yes, his code was borked.

jseban 5y ago
In this example, of course person A is completely in the wrong, but this is a bigger problem of being so immature that you can't admit any fault. My suggestion is more along the lines of: use a pairing session for review so that you can bring your empathy as well as your technical expertise, and make it a step in the process just like any other steps (testing, PO approval) etc, and just trust people to do it. I don't think there's any reason to use a tool from open source, to make code review enforced and with passive aggressive online communication and "blocking". Just seems to make work more painful, and less efficient as well actually.
SilasX 5y ago
Oh wow, kudos to you, not just for having to deal with that, and getting A removed, but for being so diplomatic about approaching him on the issue. And yes, someone that lost is pretty rare, but I'd say lower-grade versions of non-transparency and making their work hard to follow is pretty typical (and frustrating).

jseban 5y ago

In this example, of course person A is completely in the wrong, but this is a bigger problem of being so immature that you can't admit any fault. My suggestion is more along the lines of: use a pairing session for review so that you can bring your empathy as well as your technical expertise, and make it a step in the process just like any other steps (testing, PO approval) etc, and just trust people to do it. I don't think there's any reason to use a tool from open source, to make code review enforced and with passive aggressive online communication and "blocking". Just seems to make work more painful, and less efficient as well actually.

SilasX 5y ago

Oh wow, kudos to you, not just for having to deal with that, and getting A removed, but for being so diplomatic about approaching him on the issue. And yes, someone that lost is pretty rare, but I'd say lower-grade versions of non-transparency and making their work hard to follow is pretty typical (and frustrating).