3 ms·

Code review is a sticking point for a lot of the junior hires I've worked with. I now give them a talk about how code reviews are not to be taken personally, ho

Code review is a sticking point for a lot of the junior hires I've worked with. I now give them a talk about how code reviews are not to be taken personally, how they should assume best intent, and some guidance for how to handle disagreements. In my experience, setting an expectation that teams need to be respectful and communicative in code reviews, combined with no tolerance for bad behavior, is sufficient to keep the code review process running smoothly. I'm not a fan of explicit process documents that prescribe communication practices to employees. There is a very specific type of person who loves to be handed a rule book for how to communicate with others, and they love these systems. They prefer to operate within structured environments where they can read the rulebook ahead of time, or perhaps even write the rulebook for others to follow. Having the rules spelled out like this brings an artificial sense of comfort, especially to those who otherwise struggle with organic communication. For everyone else, the complexity of these systems is unnecessary mental overhead. Do two developers who were already communicating just fine need to adopt this process? More importantly, does the process stop here, or will there be additional sets of communication rules whenever another possible communication ambiguity arises?

deleted 6y ago
[deleted]
tharkun__ 6y ago
This, this, this and again this! I find it helps a lot if people's first interactions are not just in text but preferably in person but at least in video conf. The overall culture of the company plays a big part as well and so does the experience people bring with them from other companies. If your only experience is adversarial PRs then even if the new company you joined has a perfectly calm conversation in PRs you may read them as adversarial. Its the whole 'tone is not included in written communication' thing. So whatever expectation I have about how you would actually say the written thing will color everything. If we haven't had any real life interactions so far I may substitute my really bad 'last 3 companies' PR experience.
redis_mlc 6y ago
> Code review is a sticking point for a lot of the junior hires I've worked with. Well, there's always Starbucks aa a fallback. Just ask AOC.
sneak 6y ago
> combined with no tolerance for bad behavior People are terminated for a single hot-under-the-collar comment?
- bmn__ 6y ago
  Don't you think that is quite the disproportionate response? "No tolerance" simply means that bad behaviour will not be ignored, that is to say that there will be consequences.
  - sneak 6y ago
    I do think it's disproportionate, which is why I asked. "No tolerance" suggests something different to me than what you described.
gundmc 6y ago
I strongly disagree with the opposition to process documents. I've worked at many companies that functioned without good process documentation (for all functions, not just code reviews) and it simply _does not scale_ with organization size or process complexity. There's no reason it can't be both intuitive and human while also reflected in process documentation that serves as a shared reference and grounding point.
kradroy 6y ago
As a manager this is how I've laid out our code review process, almost exactly. It has worked well since we adopted it. The only two devs who have left the team in the past 6 years were the types you mentioned: the rule follower and the rule maker. The rule follower could neither handle the ambiguity of the communication media (written text) nor could he understand that the intention of some communications were not to dispute or criticize (cultural and personal communication idiosyncrasies) his work. The rule maker was just a control freak. I'm glad he's gone.
908B64B197 6y ago
> I now give them a talk about how code reviews are not to be taken personally, how they should assume best intent, and some guidance for how to handle disagreements. That's the best part. Junior hires think their code review went horribly because they had to fix a lot of things. And then their performance review says that they are doing a great job.

deleted 6y ago

[deleted]

tharkun__ 6y ago

This, this, this and again this! I find it helps a lot if people's first interactions are not just in text but preferably in person but at least in video conf. The overall culture of the company plays a big part as well and so does the experience people bring with them from other companies. If your only experience is adversarial PRs then even if the new company you joined has a perfectly calm conversation in PRs you may read them as adversarial. Its the whole 'tone is not included in written communication' thing. So whatever expectation I have about how you would actually say the written thing will color everything. If we haven't had any real life interactions so far I may substitute my really bad 'last 3 companies' PR experience.

redis_mlc 6y ago

> Code review is a sticking point for a lot of the junior hires I've worked with. Well, there's always Starbucks aa a fallback. Just ask AOC.

sneak 6y ago

> combined with no tolerance for bad behavior People are terminated for a single hot-under-the-collar comment?

bmn__ 6y ago
Don't you think that is quite the disproportionate response? "No tolerance" simply means that bad behaviour will not be ignored, that is to say that there will be consequences.
- sneak 6y ago
  I do think it's disproportionate, which is why I asked. "No tolerance" suggests something different to me than what you described.

gundmc 6y ago

I strongly disagree with the opposition to process documents. I've worked at many companies that functioned without good process documentation (for all functions, not just code reviews) and it simply _does not scale_ with organization size or process complexity. There's no reason it can't be both intuitive and human while also reflected in process documentation that serves as a shared reference and grounding point.

kradroy 6y ago

As a manager this is how I've laid out our code review process, almost exactly. It has worked well since we adopted it. The only two devs who have left the team in the past 6 years were the types you mentioned: the rule follower and the rule maker. The rule follower could neither handle the ambiguity of the communication media (written text) nor could he understand that the intention of some communications were not to dispute or criticize (cultural and personal communication idiosyncrasies) his work. The rule maker was just a control freak. I'm glad he's gone.

908B64B197 6y ago

> I now give them a talk about how code reviews are not to be taken personally, how they should assume best intent, and some guidance for how to handle disagreements. That's the best part. Junior hires think their code review went horribly because they had to fix a lot of things. And then their performance review says that they are doing a great job.