Ask HN: Does anyone dislike public code review of coworkers' code on GitHub?
7 comments
A few thoughts:
1) you are not your code. When you're doing a code review, you're critiquing the code, not the person.
2) critique is not the same thing as criticism. Yes, I was taught to criticize privately, but critique is frequently public (within a small group). Artists have critiqued publicly for centuries. The reason is that it's not only the creator who can learn from the critique.
3) critique doesn't have to come across as harsh. Instead of "you should have done Foo using Bar instead of Baz", try "why did you decide to use Bar instead of Baz for Foo?". Instead of "this code is badly written", try "do you think this might be simpler/clearer if you did it this way instead?" Personally, I've learned a ton by asking questions like these and paying attention to the answers.
It's possible (preferable, IMO) to critique with empathy. The nice thing about code review is that it's an asynchronous medium where you can think through how you phrase things, which can be good practice for learning to interact empathetically in other potentially confrontational contexts.
1) you are not your code. When you're doing a code review, you're critiquing the code, not the person.
2) critique is not the same thing as criticism. Yes, I was taught to criticize privately, but critique is frequently public (within a small group). Artists have critiqued publicly for centuries. The reason is that it's not only the creator who can learn from the critique.
3) critique doesn't have to come across as harsh. Instead of "you should have done Foo using Bar instead of Baz", try "why did you decide to use Bar instead of Baz for Foo?". Instead of "this code is badly written", try "do you think this might be simpler/clearer if you did it this way instead?" Personally, I've learned a ton by asking questions like these and paying attention to the answers.
It's possible (preferable, IMO) to critique with empathy. The nice thing about code review is that it's an asynchronous medium where you can think through how you phrase things, which can be good practice for learning to interact empathetically in other potentially confrontational contexts.
> Personally, I've learned a ton by asking questions like these and paying attention to the answers.
Yes good code reviews can be a learning experience for both reviewer and reviewee.
At work I lurk on a number of PRs that I’m directly involved with just so I can learn from the comments.
Maybe I’m lucky to be part of a team with a healthy code review culture.
Yes good code reviews can be a learning experience for both reviewer and reviewee.
At work I lurk on a number of PRs that I’m directly involved with just so I can learn from the comments.
Maybe I’m lucky to be part of a team with a healthy code review culture.
If your code review makes the coder feel personally attacked or embarrassed, you're not doing something right. Code reviews are meant for 2 (or more) way communications and discussions in order to hold quality of the code up to a standard, not a opportunity for the reviewer to feel superior and attack others. Commenting things like "this line is poorly written" is not helpful to anyone, instead try "do you x would be a better solution? because <insert reason>".
I'm sorry but if you're a functioning adult working in a professional environment, you should expect to receive critiques and feedback on your work. If someone is so sensitive that they cannot accept constructive criticism on their work, I don't know how they're ever going to learn or improve.
I'm sorry but if you're a functioning adult working in a professional environment, you should expect to receive critiques and feedback on your work. If someone is so sensitive that they cannot accept constructive criticism on their work, I don't know how they're ever going to learn or improve.
Just for clarification, when you say public do you mean open to everyone (the internet) to make a code review or open for everyone in the team to see and comment on.
Personally I find the first idea fairly bad since the amount of constructive criticisms you might get from people far outside of the problem realm seems limited. Having them open to the entire team on the other hand seems like a good way to spread knowledge and to get input from different sources
Personally I find the first idea fairly bad since the amount of constructive criticisms you might get from people far outside of the problem realm seems limited. Having them open to the entire team on the other hand seems like a good way to spread knowledge and to get input from different sources
You lose the ability to bring in additional co-workers into the discussion for important decisions by making it private.
I feel like if you're embarrassing other people in code reviews, you have a culture problem that probably won't be resolved by doing private code reviews.
You basically want to take a Postel's law type position, where you're quick to accept others' feedback and make the changes they request, while being conservative in the feedback and requests for changes you emit. Keep in mind that you're trying to get stuff done, not win debates with developers, who can be unbelievably persnickety and shortsighted.
If a reviewer asks you to change something and you believe your version is just as good as what they suggest, change it. If they suggest something they claim is "totally optional", do it. Remember, we're getting stuff done. This is faster than arguing (over stupid shit that isn't going to matter anyway).
On the other side, if you ask for something in a review and they're not gonna do it, bias toward letting that slide unless it's a real problem. If you think a piece of feedback will seem embarrassing, mention it in a private chat or something first so they have an opportunity to address it.
Code-style issues should not come up in reviews. Set up a linter, fail builds on lint errors, and forget about it.
Maybe it is difficult for introverts, but you know, a lot of things are going to be difficult. Sorry about it.
If a reviewer asks you to change something and you believe your version is just as good as what they suggest, change it. If they suggest something they claim is "totally optional", do it. Remember, we're getting stuff done. This is faster than arguing (over stupid shit that isn't going to matter anyway).
On the other side, if you ask for something in a review and they're not gonna do it, bias toward letting that slide unless it's a real problem. If you think a piece of feedback will seem embarrassing, mention it in a private chat or something first so they have an opportunity to address it.
Code-style issues should not come up in reviews. Set up a linter, fail builds on lint errors, and forget about it.
Maybe it is difficult for introverts, but you know, a lot of things are going to be difficult. Sorry about it.
DownSides: a) Potentially embarrasses the person b) Can create animosity towards co-workers c) Difficult for introverts regarding reviewing & critiquing
Alternative: Private code reviews
Any suggestions?