You shouldn’t be having visceral reactions to peoples’ coding mistakes anyway.
No. Emotions are part of how humans work. Of course, these emotions can lead to various kinds of communication, and in a team setting choosing kind and useful communication is important.
I would prefer the author say “Your code review need not contain raw emotional language.”
Agreed. And it’s not just mistakes — I remember (almost 20 years ago) being really steamed by a coworker’s commit that added a new feature by spicing a bunch of awful kludges into clean code I’d written. It felt like an insult, like sabotage. I went next door and started yelling at him. Not one of my finest moments.
At least the awkward memory will keep you from doing the same in the future, and as long as he didn’t give up and go to law school there’s probably no long-term harm ;-)
I would prefer the author say “Your code review need not contain raw emotional language.”
Outside of obvious emotive words, what makes a statement “raw emotional language”?
Who – which particular group of people (amidst an international community of tech people, where emotional appraisal varies among cultures) – gets to objectively qualify it as so?
Who … gets to objectively qualify it as [“raw emotional language”]?
You added “objective” to this question. I would not expect such judgments to be objective.
One possible rephrasing of the question is “Who gets to decide if there is raw emotional language?”. Still, that phrasing is limiting, because the question may imply:
there is (a) one person or group…
… making (b) “the decision”
However, there doesn’t have to be one person or group acting. Also, there doesn’t need to be one decision made. There is more flexibility than that.
Here is a better phrasing of a set of questions: “How would this suggestion (to avoid raw emotional language) play out? Would it be a guideline? Would it be enforced? If so, how?”. Rather than write at length, I’ll just say this becomes a decision for teams to figure out; i.e. managers, human resources, and employees.
Your better phrasing still doesn’t address the question of what makes something (such as a statement in a code review) “raw emotional language”.
In a diverse team comprised of people from multiple backgrounds across the planet, many will have differing definitions given the cultural differences in emotional appraisals (especially of language), thus there is no globally uniform notion as “raw emotional language” (beyond emotive words) that everyone will sincerely agree upon … and so ultimately – if one were to enforce, be it as a guideline or a hard rule, this nebulous idea at all (aka. “play out”) – it would have to come down to whichever person/group (usually in authority) gets to choose what is/ what is not considered “raw emotional language” (at the expense of going against a subset of their employees).
Overall, I think you are losing essential context here. Remember the context: my feedback is offered at a high level with regards to the original blog post. Please read it in this context. Nothing there is perfect, and there are no claims that enforcement should be harsh or rigid. The post does not suggest these guidelines have to be imposed.
In a diverse team comprised of people from multiple backgrounds across the planet, many will have differing definitions given the cultural differences in emotional appraisals (especially of language), thus there is no globally uniform notion as “raw emotional language” (beyond emotive words) that everyone will sincerely agree upon.
You’re making a false dichotomy here. There is likely to be significant common ground.
In areas where there may be differences of experience or opinion, having these kinds of discussion will be valuable. It is very useful for a team to be aware of how their language affects each other. It can help build common understanding and some shared norms.
Yes, even after these discussions, some areas of disagreement may remain – such is the nature of humanity. Having such differences is not a convincing justification for throwing out an imperfect goal altogether.
this nebulous idea at all (aka. “play out”) – it would have to come down to whichever person/group (usually in authority) gets to choose what is/ what is not considered “raw emotional language” (at the expense of going against a subset of their employees).
It sounds to me like you are asking for the impossible – a completely unambiguous definition, applicable across all regions on earth, where all employees sincerely agree at a particular time.* I think it would be helpful if you accept that this is not going to happen perfectly, and this is ok. However, it is still better to strive to not include raw emotional language in a code review. Some imperfections in figuring this out do not justify throwing out the goal of being calm and rational in a code review.
I do understand that you are giving a high level and general view … however my comment is primarily in response to a very specific code review criterion proposed (be it prescribed as a guideline or imposed as a hard/ rigid rule), viz:
Your code review need not contain raw emotional language.
which you also repeat but in a different form as,
it is still better to strive to not include raw emotional language in a code review
Hence my as yet unanswered query of just what would constitute this “raw emotional language” (beyond emotive words).
Now if, as you say, there is likely to be “significant common ground” in regards to emotional appraisal of statements in code reviews among a diverse team comprised of people from multiple backgrounds across the planet – such as from the Americas, Europe, Africa, Asia, Russia, Australia, etc – then surely it would be straightforward to directly answer that as yet answered query?
It sounds to me like you are asking for the impossible – a completely unambiguous definition, applicable across all regions on earth, where all employees sincerely agree at a particular time.
Which is why I say that if one were to prescribe this as a guideline or impose it as hard rule – it would ultimately have to come down to whichever person/group (usually in authority*) gets to choose what is/ what is not considered “raw emotional language” (at the expense of going against the rest). However if it be as loose and time-varying in meaning as you say it is, then nobody nowhere at no point in time will have any idea of what “raw emotional language” is, thus making the whole guideline/ rule null and void to begin with.
* I use this word in its sociological meaning (and regardless of organizational hierarchies).
However if it be as loose and time-varying in meaning as you say it is…
You are misrepresenting what I wrote.
… then nobody nowhere at no point in time will have any idea of what “raw emotional language” is, thus making the whole guideline/ rule null and void to begin with.
No.
In general, you sound uncomfortable with any principle that isn’t perfectly clear and agreed to by all people in a team. Fair?
Also, it sounds like you want me to elaborate on what “raw emotional language” means for a diverse team of people across continents. My response is simple:
My argument does not hinge on my ability to do that in general (for some arbitrary team I’ve never interacted with).
Instead, I believe a (mostly) reasonable group of people can figure this out.
Some complexity and disagreement is to be expected when people are involved. This is ok. It is better to have a broadly useful guideline, even if imperfect. Its imperfection does not render it “null and void”. Rather, it means that humans have to figure things out.
It is important for teams to socialize some norms and figure out ways to encourage/enforce them.* Yes, all other things equal, clear norms are better than vague norms. That said, the presence of disagreement is not a sign that the norms are failing. Rather, it is a sign that people have different interests and are finding ways to work together.
* Yes, there is a spectrum. Some teams may prefer more norms, and some may prefer fewer. I’m not attempting to specify what is “correct” for every team.
It sounds to me like you are asking for the impossible – a completely unambiguous definition, applicable across all regions on earth, where all employees sincerely agree at a particular time (Remember that people can adapt and change)
However if it be as loose and time-varying in meaning as you say it is…
You are misrepresenting what I wrote.
Your previous comment (quoted above) implies that the definition of “raw emotional language” is not unambiguous (hence “loose in meaning”), and that its meaning can change over time as people can adapt and change over time (hence time-varying).
… then nobody nowhere at no point in time will have any idea of what “raw emotional language” is, thus making the whole guideline/ rule null and void to begin with.
No.
So on what basis would you say that a new member of the team, half way across the world, while doing their code review (and feeling good or neutral whilst doing so), appraise what they are about to write as “raw emotional language” or not?
In general, you sound uncomfortable with any principle that isn’t perfectly clear and agreed to by all people in a team.
Not any principle, but a principle that uses an invented phrase with no commonly agreed upon meaning.
Now I understand the intent behind such a principle (to facilitate a friendly environment for all) - however without commonly agreed upon meaning, this sort of guideline is ripe for abuse, depending who holds the power in a team. I actually agree with you that “raw emotional language” cannot be precisely defined (it is mostly a subjective thing; varies between cultures; and has no strong association with the actual emotion expressed and spread as vibes), which is why I say that if one were to prescribe this as a guideline or impose it as hard rule – it would ultimately have to come down to whichever person/group (usually in authority) gets to choose, at whatever point in time, what is/ what is not considered “raw emotional language” (at the expense of going against the rest).
There are far better ways to approach this issue than imposing introjective overhead on globally diverse team members.
I’ll put aside the misrepresentation issue. I’ll just call it a misunderstanding.
however without commonly agreed upon meaning, this sort of guideline is ripe for abuse
No, in general, one should not assume an imperfect norm is ripe for abuse. This is cherry picking one possible outcome. Another possible outcome is that a somewhat flexible norm is more accommodating. Simply put, it depends.
But, yes, a group should figure out what it’s norms are over time. This happens all the time.
You see to have a rather pessimistic view of this kind of situation. You see to accentuate only the potential downsides.
But, of course, groups figure out norms regularly. Often with overall success.
Yes, some people in power abuse rules to their advantage. This is often a function of their power imbalance. In my view, ethically challenged people in power don’t ‘need’ a somewhat ambiguous norm to abuse their power. They can find any number of ways to do it.
Some people are pessimistic about things and that’s fine. But please don’t mistake that pessimism for fact. Consider taking a broader view of pros and cons.
There are far better ways to approach this issue…
Please define the issue and the problem as you see it. Then please share some of the better ways you see.
You see to have a rather pessimistic view of this kind of situation. You see to accentuate only the potential downsides.
I’m actually quite optimistic about code review communication, and have no trouble with them. The downside is only when you deviate from what demonstrably works. I have not seen any demonstrable evidence that “having visceral reactions to peoples’ coding mistakes” for instance (which is what prompted this whole comment thread) is minimized collectively in a diverse team spanning the globe by institutionalizing vague behavioral guidelines such as not to use “raw emotional language”.
Please define the issue and the problem as you see it.
The issue is conflict (thus an absence of friendly ambiance), part of which is “having visceral reactions to peoples’ coding mistakes”.
Then please share some of the better ways you see.
If A is having a “visceral reaction” to B’s statement, and if it is not clear that B is feeling malicious (either peeved or angry) - then it is A’s responsibility to try to better understand what’s happening. Now I disagree with the post that A “shouldn’t be having” these reactions (inasmuch as affect is often an automatic response). A doesn’t have total control over their feeling patterns (nobody does), but what they can do is communicate with B as a fellow human being to clarify their intent/stance so that the chances of these “visceral reactions” are minimized. The submitted post also segues to it in another roadblock context: “After dozens of contrasting PR comments and proposed solutions, it should be clear that online communication has become unproductive for the issue at hand. Send a meeting hit to involved members to discuss the issue offline.”
Besides, one can adjust their “language” all day along, and yet malice (either peeved or angry) will seep through it anyway,[1] causing “visceral reactions” on their team members (this is what the sixties’ term “vibes” refers to). It is better to establish a culture of communication and understanding from the get go.
[1]: For instance
The leadership style in Elm is extremely aggressive and authoritarian.
By that I do not mean impolite or rude. It is almost always very civil. But still ultimately aggressive and controlling. — Luke Plant
Or, for another instance:
… once enough people are piling on to complain or tell you what’s wrong with what you did, you’re going to feel attacked - even if every single comment is worded politely and respectfully. — SO
or:
If only I could have read this years ago, before I had the misfortune of working at NoRedInk. The team shows a bewildering mix of cargo-cult inclusiveness coupled with inability to consider that anyone could be different from them in any way that matters. I’ve never before or since met a group where a mentor would advise someone to “show more curiosity” by asking fewer questions!
[I was] fired halfway through the period for “engaging in conflict” which I can only speculate referred to a bizarre conversation on company Slack about whether or not it made sense to add a cache. — former employee:
Ok, now that you’ve shared what you think is a better alternative, I understand your POV better. From my POV, what you write in the second half of your comment makes sense.
It still seems to me that you were not understanding what I was trying to get across. So, let me clarify something…
I have not seen any demonstrable evidence that “having visceral reactions to peoples’ coding mistakes” for instance (which is what prompted this whole comment thread) is minimized collectively in a diverse team spanning the globe by institutionalizing vague behavioral guidelines such as not to use “raw emotional language”.
Ah ha!… I may now see why this conversation has felt so murky (i.e. I could not figure out what was bothering you).
Let’s clear this up – I didn’t say that “having visceral reactions to people’s coding mistakes” is minimized by having a guideline such as avoiding “raw emotional language”. (You seem to have made a connection between these ideas that I did not say or intend.)
Instead, my argument goes like this (in context of a code review);
Humans naturally sometime have visceral reactions
This is ok – but take enough time to pause and reflect before writing a response.
When you do respond, it is a good idea not to use raw emotional language.
Calling reviewers’ behavior “toxic behavior” seems pretty toxic itself. It immediately puts it in an us-vs-them hostile framing. If “toxic” is removed from the title, it would be less likely to rile people up. The “unhelpful” framing is better, but still violates the guideline against passing opinion off as fact. How about “Review behaviors I think are counterproductive”?
I think “toxic” is a pretty reasonable descriptor in this context. Behaviors which are unhelpful, counterproductive, adversarial, and have a net negative effect on culture and trust are IMO bad enough to be classified as “toxic”, especially since they have a tendency to spread around unchecked.
Yes, the word “toxic” can be misused. It can be used to bash people over the head, overgeneralize, and proclaim one’s self-importance, none of which tend to be useful.
Still, I think there is a useful core to the word: calling attention to individual behaviors that might be narrowly defensible (e.g. technically correct) while also being harmful to a group.
I would agree it’s widely misused where “harmful” would do (but that typically requires being willing & able to enunciate the specific harm, whereas “toxic” tends - for whatever reason - to get a pass on that front).
Fair enough. Whatever we choose – “toxic” or “unhelpful” – these words tend to imply a value judgment about what is important to a group.
It immediately puts it in an us-vs-them hostile framing.
Well said. It depends on it a value is shared between two people:
On one hand, if two people share a value, pointing out a value misalignment may be useful. The other person might think “I value X, and I didn’t mean to take an action that compromised it.” and be willing to discuss and/or change behavior.
On the other hand, if two people disagree on a value (or its relative importance), calling out another person for failing to live up to a value often starts a conversation on the wrong foot.*
* In my view, it takes a certain patience and skill to be insulted or judged by another person, and calmly understand the other person’s motivations without being bothered. There are great opportunities to turn a situation around. Hostile people often are surprised by people that engage without cowering or yelling back.
Was a bit surprised to not see this helpful thing mentioned:
Add positive feedback as well.
It feels that adding “great tests”, “whoa, didn’t think such a nice solution is possible”, “nice idiom!”, etc comments (where they are true, of course), makes the overall review more productive. Thinking fast and slow also mentions that people learn better when rewarded for positive behavior, than when punished for negative.
I’m guessing it is a reaction to the language and/or mentailty presented in the article, that can be percieved as “HR”/buzz word-y. I can relate to it, even if I don’t think it is off-topic.
Unhelpful behavior #4: asking judgmental questions
Avoid asking judgmental questions like:
Why didn’t you just do ___ here?”
If I trust your programming ability and you do something non-obvious, I’m going to ask why because I’m going to assume you have a good reason and I would like to learn.
I think she’s only talking about the flip side of that, where you’re blaming someone for doing it wrong.
It occurs to me that it can be hard to tell these apart. So when asking the sincere version you refer to, one should probably try to word it less ambiguously. Maybe “Just curious why you’re doing this instead of ___?” Although maybe that just comes off as subtler sarcasm. Sigh, communication is hard.
Yes, hedge words can be very helpful. Also: I-statements, opinion rather than fact, approaching with curiosity and uncertainty rather than confident declaration… And yeah, this can require writing more words and taking more time to phrase things, but I’ve found it easier over time. I find myself writing things like “I think I would normally prefer X here – is there some benefit to this approach instead?” If that comes across as sarcasm, there’s probably an existing trust problem that needs addressing.
(Sometimes it turns out their code has some hidden upside, in which case I gently suggest a quick code comment or even a test, if appropriate. If I’m asking as a reviewer, the next person to touch the code will probably wonder as well.)
Another approach that I’ve seen is to prefix each comment with an explicit description of what kind of comment it is: [seeking clarity] Why is … / [suggesting alternative] What about … / [request] Can you add a comment about why …
All this said, I’ve only ever encountered one person who I’ve had a bad interaction with during code reviews in either direction. The people I’ve worked with have been very mature and empathetic. There’s one person who has done one of the behaviors on the list, but not enough to bother me greatly, and I believe that if I push back a bit they’ll be happy to discuss it and work out shared norms. So uh, also “work with nice people”. Pro tip.
If it’s not obvious why something is being done, or why it’s being done a certain way, I often point out that it is deserving of a code comment. This offers the opportunity to reflect on whether it’s a good idea or fundamentally unclear and in need of documentation, without being overtly judgmental.
An experienced developer will probably rethink whether the code can be a bit clearer, but a junior/less experienced developer (or an experienced developer who is distracted/tired/etc) might take it at face value and just document the code (which sometimes is the best solution, of course, but certainly not always).
There are much better ways of asking that doesn’t use the judgemental “why didn’t you just ___” formulation, and also shows your intent (wanting to learn) much better. “Please help me understand why you did this instead of __” for example. Or “Did you consider __?” if brevity is key. (It’s even shorter than the original.)
Quite a few of these points are about expectations and interpretations; for example is “this component should be stateless” really passing off opinion as fact? Well, depends on how you read it; personally I’d think that the “I think it would be better if” is implied in a good-faith interpretation of it, and doesn’t really need to be typed out. The same applies to “judgemental” questions like “why didn’t you just do ___ here?”.
If it’s a stranger on a random open source project then I tend to be a bit more verbose to avoid misunderstandings. If it’s a coworker with whom I have a good repertoire then I can be more concise and brief. There is value in being concise too; less work for me to type, and less work for you to read.
Looking for the worst possible meaning in comments is also toxic.
It’s good to be aware of how our words impact others, but it’s also important to not forget about technical precision just because we are in consideration of our emotions. I mostly want to add nuance about unhelpful behaviors.
In #1, will the suggested change improve performance? To prove it, make the change locally and run the measurement. We should be careful to not simply take our opinions and phrase them with extra words in the hope that they will sound less opinionated; we should avoid rationalizing. The author does provide the useful advice (helpful behavior #7, in fact) of canonicalizing opinions, using tools like linters and formatters to remove opportunities for bikeshedding.
In #3, I think that it is worth asking why any code maintenance team would tolerate maintenance of messy code. Preventative maintenance can massively lower the costs of working on a codebase; I was raised with the adage, “always leave camp cleaner than you found it.” It is indeed unfair to ask people to probe into nasty modules in order to surgically implement a single feature or bugfix; however, it is also unfair to check in such nasty modules in the first place, and code review is the only place where such nastiness can be examined before check-in.
In #5, we should try to understand sarcasm as a coping mechanism for the fact that we will never produce anything perfect, and the accumulation of errata is not just inevitable, but bizarre. I cannot find the precise phrasing, but I recall a datasheet which documented a register as taking values from 0 to 7 (3 bits), and helpfully indicated that if one ever wanted to write 7 to the register, then one should just write 6 instead, because the hardware always turned 6 into 7. I don’t think that I have ever been able to engage with this without at least a little sarcasm. That said, the examples given are not just sarcastic about software, but actively attack contributors, which is inexcusable.
In #6, I think that #5 should be applied. Emoji are very easy to misinterpret; famous examples include ‘PISTOL’ (U+1F52B) and ‘AUBERGINE’ (U+1F346), popularly known as the “gun emoji” and “eggplant emoji” respectively. Signifying that code looks good or giving praise with emoji is just as susceptible to misinterpretation via sarcastic usage, like with the “hooray emoji” ‘PARTY POPPER’ (U+1F389) on GitHub.
In #7, this is a UX problem on GitHub. I have used code review systems which track each individual comment in a pull request as a living note which updates as new changes are added. In these systems, it is not ambiguous whether a comment has been addressed; each change explicitly comes with the opportunity for the author to acknowledge feedback.
I would consider another helpful behavior: When a review gets long, add an introductory paragraph. Even a single sentence can set the tone and expectations for the author reading the review, and keep otherwise-painfully-dry nitpicking from being rude.
I didn’t reply to every section because some sections had no technical problems whatsoever, and I agree with the non-technical thrust of the article. LGTM 🎉
In #5, we should try to understand sarcasm as a coping mechanism for the fact that we will never produce anything perfect, and the accumulation of errata is not just inevitable, but bizarre.
As a very verbal person who’s quick with a comeback, I learned early that deploying sarcasm can be incredibly satisfying - and deeply hurtful. It’s a bit like terms of endearment - best reserved for people you know very well.
If you notice someone being unhelpful during code reviews, consider letting them know (if you feel safe in your role/company doing so) and be direct.
My experience : I’ve dealt this with, and it was difficult. The abrasive person had more seniority (based on time served at the organization, not based on experience).
My personal failing: #3, asking people to fix something they didn’t cause because I happened to spot it adjacent to their change in their PR. I shall endeavour to stop this.
For number 4, I often comment freely during the review, but look over my comments again before submitting. If it feels like a lot of comments, I often delete the ones I care less about and focus on a few that I think have the biggest impact. Often addressing those will naturally address the minor comments as well.
Since this component doesn’t have any lifecycle methods or state, it could be made a stateless functional component. This will improve performance and readability. Here is some documentation.
This just rubs me the wrong way. The revised comment doesn’t actually ask the person to do anything. And if you’re not going to ask them to do anything, then why leave a comment at all? I think it’s important to actually make it apparent that this is something you’re flagging and would like to see fixed in order to approve their PR. Not to mention, doesn’t this just make “This will improve performance and readability” the part that’s passing off an opinion as fact?
“Could” is such a meaningless, wishy-washy word here. Of course it could - are you telling me you think it’s better? Are you just throwing out a random option with some documentation you want me to read, while not being totally invested anyway, and you want me to defend what I’ve done?
It feels a bit condescending if anything, like “oh, I guess I just wasn’t informed enough” or else I would have known about this obviously better approach.
But anyway, if the author here takes issue with the idea of stating opinions as facts, then perhaps a better way is a simple:
Please make this component stateless.
At other times, it’s appropriate to not make a direct request, perhaps when you’re somewhat junior or you’re not positive yourself and just brainstorming a suggestion. But then, contrary to the article’s version, you should make it explicit that it’s a suggestion and you’re soliciting feedback:
What do you think about making this component stateless? Often times that improves performance and readability, though I’m not positive that’s the case here.
I like to think of it in terms of “emotional labor”. Basically, make it easy on the author to respond to your PR, and don’t waste their time. If you feel strongly that your way is better, make the explicit request and the person can simply implement that change without thinking about it too much, even if they’re personally on the fence about whether it’s actually better. If you’re really not sure, leave it a question or give them an easy out, so the person can choose to not make your change quickly. But don’t leave it ambiguous about how invested you are in the comment. “This could have been done this way” is not super useful, less so with a bunch of documentation to read. Now the author is in the position of trying to infer how invested you are in the comment, whether you actually are asking for a change, or asking a question about their approach vs yours, and whether you should try to defend yours or what. There’s rarely an unequivocally “best” approach, and so an informational request-less comment invites the person to try to re-litigate for themselves the pros and cons of two different approaches they likely already considered.
Github’s “suggest” feature where you can simply suggest a fix for a number of lines and the author can commit that change from the web UI can save you from trying to phrase your change suggestion in words and you have an actual snippet of code to discuss further.
No. Emotions are part of how humans work. Of course, these emotions can lead to various kinds of communication, and in a team setting choosing kind and useful communication is important.
I would prefer the author say “Your code review need not contain raw emotional language.”
P.S. The word should is a minefield in writing.
Agreed. And it’s not just mistakes — I remember (almost 20 years ago) being really steamed by a coworker’s commit that added a new feature by spicing a bunch of awful kludges into clean code I’d written. It felt like an insult, like sabotage. I went next door and started yelling at him. Not one of my finest moments.
At least the awkward memory will keep you from doing the same in the future, and as long as he didn’t give up and go to law school there’s probably no long-term harm ;-)
Outside of obvious emotive words, what makes a statement “raw emotional language”?
Who – which particular group of people (amidst an international community of tech people, where emotional appraisal varies among cultures) – gets to objectively qualify it as so?
You added “objective” to this question. I would not expect such judgments to be objective.
One possible rephrasing of the question is “Who gets to decide if there is raw emotional language?”. Still, that phrasing is limiting, because the question may imply:
However, there doesn’t have to be one person or group acting. Also, there doesn’t need to be one decision made. There is more flexibility than that.
Here is a better phrasing of a set of questions: “How would this suggestion (to avoid raw emotional language) play out? Would it be a guideline? Would it be enforced? If so, how?”. Rather than write at length, I’ll just say this becomes a decision for teams to figure out; i.e. managers, human resources, and employees.
Your better phrasing still doesn’t address the question of what makes something (such as a statement in a code review) “raw emotional language”.
In a diverse team comprised of people from multiple backgrounds across the planet, many will have differing definitions given the cultural differences in emotional appraisals (especially of language), thus there is no globally uniform notion as “raw emotional language” (beyond emotive words) that everyone will sincerely agree upon … and so ultimately – if one were to enforce, be it as a guideline or a hard rule, this nebulous idea at all (aka. “play out”) – it would have to come down to whichever person/group (usually in authority) gets to choose what is/ what is not considered “raw emotional language” (at the expense of going against a subset of their employees).
Overall, I think you are losing essential context here. Remember the context: my feedback is offered at a high level with regards to the original blog post. Please read it in this context. Nothing there is perfect, and there are no claims that enforcement should be harsh or rigid. The post does not suggest these guidelines have to be imposed.
You’re making a false dichotomy here. There is likely to be significant common ground.
In areas where there may be differences of experience or opinion, having these kinds of discussion will be valuable. It is very useful for a team to be aware of how their language affects each other. It can help build common understanding and some shared norms.
Yes, even after these discussions, some areas of disagreement may remain – such is the nature of humanity. Having such differences is not a convincing justification for throwing out an imperfect goal altogether.
It sounds to me like you are asking for the impossible – a completely unambiguous definition, applicable across all regions on earth, where all employees sincerely agree at a particular time.* I think it would be helpful if you accept that this is not going to happen perfectly, and this is ok. However, it is still better to strive to not include raw emotional language in a code review. Some imperfections in figuring this out do not justify throwing out the goal of being calm and rational in a code review.
* Remember that people can adapt and change.
I do understand that you are giving a high level and general view … however my comment is primarily in response to a very specific code review criterion proposed (be it prescribed as a guideline or imposed as a hard/ rigid rule), viz:
which you also repeat but in a different form as,
Hence my as yet unanswered query of just what would constitute this “raw emotional language” (beyond emotive words).
Now if, as you say, there is likely to be “significant common ground” in regards to emotional appraisal of statements in code reviews among a diverse team comprised of people from multiple backgrounds across the planet – such as from the Americas, Europe, Africa, Asia, Russia, Australia, etc – then surely it would be straightforward to directly answer that as yet answered query?
Which is why I say that if one were to prescribe this as a guideline or impose it as hard rule – it would ultimately have to come down to whichever person/group (usually in authority*) gets to choose what is/ what is not considered “raw emotional language” (at the expense of going against the rest). However if it be as loose and time-varying in meaning as you say it is, then nobody nowhere at no point in time will have any idea of what “raw emotional language” is, thus making the whole guideline/ rule null and void to begin with.
* I use this word in its sociological meaning (and regardless of organizational hierarchies).
You are misrepresenting what I wrote.
No.
In general, you sound uncomfortable with any principle that isn’t perfectly clear and agreed to by all people in a team. Fair?
Also, it sounds like you want me to elaborate on what “raw emotional language” means for a diverse team of people across continents. My response is simple:
My argument does not hinge on my ability to do that in general (for some arbitrary team I’ve never interacted with).
Instead, I believe a (mostly) reasonable group of people can figure this out.
Some complexity and disagreement is to be expected when people are involved. This is ok. It is better to have a broadly useful guideline, even if imperfect. Its imperfection does not render it “null and void”. Rather, it means that humans have to figure things out.
It is important for teams to socialize some norms and figure out ways to encourage/enforce them.* Yes, all other things equal, clear norms are better than vague norms. That said, the presence of disagreement is not a sign that the norms are failing. Rather, it is a sign that people have different interests and are finding ways to work together.
* Yes, there is a spectrum. Some teams may prefer more norms, and some may prefer fewer. I’m not attempting to specify what is “correct” for every team.
Your previous comment (quoted above) implies that the definition of “raw emotional language” is not unambiguous (hence “loose in meaning”), and that its meaning can change over time as people can adapt and change over time (hence time-varying).
So on what basis would you say that a new member of the team, half way across the world, while doing their code review (and feeling good or neutral whilst doing so), appraise what they are about to write as “raw emotional language” or not?
Not any principle, but a principle that uses an invented phrase with no commonly agreed upon meaning.
Now I understand the intent behind such a principle (to facilitate a friendly environment for all) - however without commonly agreed upon meaning, this sort of guideline is ripe for abuse, depending who holds the power in a team. I actually agree with you that “raw emotional language” cannot be precisely defined (it is mostly a subjective thing; varies between cultures; and has no strong association with the actual emotion expressed and spread as vibes), which is why I say that if one were to prescribe this as a guideline or impose it as hard rule – it would ultimately have to come down to whichever person/group (usually in authority) gets to choose, at whatever point in time, what is/ what is not considered “raw emotional language” (at the expense of going against the rest).
There are far better ways to approach this issue than imposing introjective overhead on globally diverse team members.
I’ll put aside the misrepresentation issue. I’ll just call it a misunderstanding.
No, in general, one should not assume an imperfect norm is ripe for abuse. This is cherry picking one possible outcome. Another possible outcome is that a somewhat flexible norm is more accommodating. Simply put, it depends.
But, yes, a group should figure out what it’s norms are over time. This happens all the time.
You see to have a rather pessimistic view of this kind of situation. You see to accentuate only the potential downsides.
But, of course, groups figure out norms regularly. Often with overall success.
Yes, some people in power abuse rules to their advantage. This is often a function of their power imbalance. In my view, ethically challenged people in power don’t ‘need’ a somewhat ambiguous norm to abuse their power. They can find any number of ways to do it.
Some people are pessimistic about things and that’s fine. But please don’t mistake that pessimism for fact. Consider taking a broader view of pros and cons.
Please define the issue and the problem as you see it. Then please share some of the better ways you see.
I’m actually quite optimistic about code review communication, and have no trouble with them. The downside is only when you deviate from what demonstrably works. I have not seen any demonstrable evidence that “having visceral reactions to peoples’ coding mistakes” for instance (which is what prompted this whole comment thread) is minimized collectively in a diverse team spanning the globe by institutionalizing vague behavioral guidelines such as not to use “raw emotional language”.
The issue is conflict (thus an absence of friendly ambiance), part of which is “having visceral reactions to peoples’ coding mistakes”.
If A is having a “visceral reaction” to B’s statement, and if it is not clear that B is feeling malicious (either peeved or angry) - then it is A’s responsibility to try to better understand what’s happening. Now I disagree with the post that A “shouldn’t be having” these reactions (inasmuch as affect is often an automatic response). A doesn’t have total control over their feeling patterns (nobody does), but what they can do is communicate with B as a fellow human being to clarify their intent/stance so that the chances of these “visceral reactions” are minimized. The submitted post also segues to it in another roadblock context: “After dozens of contrasting PR comments and proposed solutions, it should be clear that online communication has become unproductive for the issue at hand. Send a meeting hit to involved members to discuss the issue offline.”
Besides, one can adjust their “language” all day along, and yet malice (either peeved or angry) will seep through it anyway,[1] causing “visceral reactions” on their team members (this is what the sixties’ term “vibes” refers to). It is better to establish a culture of communication and understanding from the get go.
[1]: For instance
Or, for another instance:
or:
Ok, now that you’ve shared what you think is a better alternative, I understand your POV better. From my POV, what you write in the second half of your comment makes sense.
It still seems to me that you were not understanding what I was trying to get across. So, let me clarify something…
Ah ha!… I may now see why this conversation has felt so murky (i.e. I could not figure out what was bothering you).
Let’s clear this up – I didn’t say that “having visceral reactions to people’s coding mistakes” is minimized by having a guideline such as avoiding “raw emotional language”. (You seem to have made a connection between these ideas that I did not say or intend.)
Instead, my argument goes like this (in context of a code review);
Humans naturally sometime have visceral reactions
This is ok – but take enough time to pause and reflect before writing a response.
When you do respond, it is a good idea not to use raw emotional language.
Hopefully this clears it up.
What?
What is it exactly that you are asking here?
Calling reviewers’ behavior “toxic behavior” seems pretty toxic itself. It immediately puts it in an us-vs-them hostile framing. If “toxic” is removed from the title, it would be less likely to rile people up. The “unhelpful” framing is better, but still violates the guideline against passing opinion off as fact. How about “Review behaviors I think are counterproductive”?
I think “toxic” is a pretty reasonable descriptor in this context. Behaviors which are unhelpful, counterproductive, adversarial, and have a net negative effect on culture and trust are IMO bad enough to be classified as “toxic”, especially since they have a tendency to spread around unchecked.
Real-world example of this in a general context (not just code reviews).
Much like “problematic,” the word “toxic” seems to have become a signifier of a certain kind of holier-than-thou preaching, and, well…. problematic.
Yes, the word “toxic” can be misused. It can be used to bash people over the head, overgeneralize, and proclaim one’s self-importance, none of which tend to be useful.
Still, I think there is a useful core to the word: calling attention to individual behaviors that might be narrowly defensible (e.g. technically correct) while also being harmful to a group.
I would agree it’s widely misused where “harmful” would do (but that typically requires being willing & able to enunciate the specific harm, whereas “toxic” tends - for whatever reason - to get a pass on that front).
Fair enough. Whatever we choose – “toxic” or “unhelpful” – these words tend to imply a value judgment about what is important to a group.
Well said. It depends on it a value is shared between two people:
On one hand, if two people share a value, pointing out a value misalignment may be useful. The other person might think “I value X, and I didn’t mean to take an action that compromised it.” and be willing to discuss and/or change behavior.
On the other hand, if two people disagree on a value (or its relative importance), calling out another person for failing to live up to a value often starts a conversation on the wrong foot.*
* In my view, it takes a certain patience and skill to be insulted or judged by another person, and calmly understand the other person’s motivations without being bothered. There are great opportunities to turn a situation around. Hostile people often are surprised by people that engage without cowering or yelling back.
Was a bit surprised to not see this helpful thing mentioned:
It feels that adding “great tests”, “whoa, didn’t think such a nice solution is possible”, “nice idiom!”, etc comments (where they are true, of course), makes the overall review more productive. Thinking fast and slow also mentions that people learn better when rewarded for positive behavior, than when punished for negative.
I don’t understand why people are voting this as off topic.
People don’t realize that software is made of people? :-)
I’m guessing it is a reaction to the language and/or mentailty presented in the article, that can be percieved as “HR”/buzz word-y. I can relate to it, even if I don’t think it is off-topic.
It uses words like “toxic” and discusses developers’ emotional states, which to some people is a signifier of the ways of the Bad Tribe™.
If I trust your programming ability and you do something non-obvious, I’m going to ask why because I’m going to assume you have a good reason and I would like to learn.
I think she’s only talking about the flip side of that, where you’re blaming someone for doing it wrong.
It occurs to me that it can be hard to tell these apart. So when asking the sincere version you refer to, one should probably try to word it less ambiguously. Maybe “Just curious why you’re doing this instead of ___?” Although maybe that just comes off as subtler sarcasm. Sigh, communication is hard.
Yes, hedge words can be very helpful. Also: I-statements, opinion rather than fact, approaching with curiosity and uncertainty rather than confident declaration… And yeah, this can require writing more words and taking more time to phrase things, but I’ve found it easier over time. I find myself writing things like “I think I would normally prefer X here – is there some benefit to this approach instead?” If that comes across as sarcasm, there’s probably an existing trust problem that needs addressing.
(Sometimes it turns out their code has some hidden upside, in which case I gently suggest a quick code comment or even a test, if appropriate. If I’m asking as a reviewer, the next person to touch the code will probably wonder as well.)
Another approach that I’ve seen is to prefix each comment with an explicit description of what kind of comment it is: [seeking clarity] Why is … / [suggesting alternative] What about … / [request] Can you add a comment about why …
All this said, I’ve only ever encountered one person who I’ve had a bad interaction with during code reviews in either direction. The people I’ve worked with have been very mature and empathetic. There’s one person who has done one of the behaviors on the list, but not enough to bother me greatly, and I believe that if I push back a bit they’ll be happy to discuss it and work out shared norms. So uh, also “work with nice people”. Pro tip.
I think the keyword is just
“Why did you do it this way?” vs. “Why didn’t you just do it this other way?” are extremely different questions.
This. “The Gentle Art of Verbal Self Defense” devotes an entire chapter to “just”.
If it’s not obvious why something is being done, or why it’s being done a certain way, I often point out that it is deserving of a code comment. This offers the opportunity to reflect on whether it’s a good idea or fundamentally unclear and in need of documentation, without being overtly judgmental.
An experienced developer will probably rethink whether the code can be a bit clearer, but a junior/less experienced developer (or an experienced developer who is distracted/tired/etc) might take it at face value and just document the code (which sometimes is the best solution, of course, but certainly not always).
There are much better ways of asking that doesn’t use the judgemental “why didn’t you just ___” formulation, and also shows your intent (wanting to learn) much better. “Please help me understand why you did this instead of __” for example. Or “Did you consider __?” if brevity is key. (It’s even shorter than the original.)
Quite a few of these points are about expectations and interpretations; for example is “this component should be stateless” really passing off opinion as fact? Well, depends on how you read it; personally I’d think that the “I think it would be better if” is implied in a good-faith interpretation of it, and doesn’t really need to be typed out. The same applies to “judgemental” questions like “why didn’t you just do ___ here?”.
If it’s a stranger on a random open source project then I tend to be a bit more verbose to avoid misunderstandings. If it’s a coworker with whom I have a good repertoire then I can be more concise and brief. There is value in being concise too; less work for me to type, and less work for you to read.
Looking for the worst possible meaning in comments is also toxic.
I tends to write such comments as “If you remove ___ this function will not affect/be affected by behaviour/changes in ___”.
It’s good to be aware of how our words impact others, but it’s also important to not forget about technical precision just because we are in consideration of our emotions. I mostly want to add nuance about unhelpful behaviors.
In #1, will the suggested change improve performance? To prove it, make the change locally and run the measurement. We should be careful to not simply take our opinions and phrase them with extra words in the hope that they will sound less opinionated; we should avoid rationalizing. The author does provide the useful advice (helpful behavior #7, in fact) of canonicalizing opinions, using tools like linters and formatters to remove opportunities for bikeshedding.
In #3, I think that it is worth asking why any code maintenance team would tolerate maintenance of messy code. Preventative maintenance can massively lower the costs of working on a codebase; I was raised with the adage, “always leave camp cleaner than you found it.” It is indeed unfair to ask people to probe into nasty modules in order to surgically implement a single feature or bugfix; however, it is also unfair to check in such nasty modules in the first place, and code review is the only place where such nastiness can be examined before check-in.
In #5, we should try to understand sarcasm as a coping mechanism for the fact that we will never produce anything perfect, and the accumulation of errata is not just inevitable, but bizarre. I cannot find the precise phrasing, but I recall a datasheet which documented a register as taking values from 0 to 7 (3 bits), and helpfully indicated that if one ever wanted to write 7 to the register, then one should just write 6 instead, because the hardware always turned 6 into 7. I don’t think that I have ever been able to engage with this without at least a little sarcasm. That said, the examples given are not just sarcastic about software, but actively attack contributors, which is inexcusable.
In #6, I think that #5 should be applied. Emoji are very easy to misinterpret; famous examples include ‘PISTOL’ (U+1F52B) and ‘AUBERGINE’ (U+1F346), popularly known as the “gun emoji” and “eggplant emoji” respectively. Signifying that code looks good or giving praise with emoji is just as susceptible to misinterpretation via sarcastic usage, like with the “hooray emoji” ‘PARTY POPPER’ (U+1F389) on GitHub.
In #7, this is a UX problem on GitHub. I have used code review systems which track each individual comment in a pull request as a living note which updates as new changes are added. In these systems, it is not ambiguous whether a comment has been addressed; each change explicitly comes with the opportunity for the author to acknowledge feedback.
I would consider another helpful behavior: When a review gets long, add an introductory paragraph. Even a single sentence can set the tone and expectations for the author reading the review, and keep otherwise-painfully-dry nitpicking from being rude.
I didn’t reply to every section because some sections had no technical problems whatsoever, and I agree with the non-technical thrust of the article. LGTM 🎉
As a very verbal person who’s quick with a comeback, I learned early that deploying sarcasm can be incredibly satisfying - and deeply hurtful. It’s a bit like terms of endearment - best reserved for people you know very well.
My experience : I’ve dealt this with, and it was difficult. The abrasive person had more seniority (based on time served at the organization, not based on experience).
My personal failing: #3, asking people to fix something they didn’t cause because I happened to spot it adjacent to their change in their PR. I shall endeavour to stop this.
For number 4, I often comment freely during the review, but look over my comments again before submitting. If it feels like a lot of comments, I often delete the ones I care less about and focus on a few that I think have the biggest impact. Often addressing those will naturally address the minor comments as well.
Interesting post. I kind of disagree that
is worse than their proposed fix of
This just rubs me the wrong way. The revised comment doesn’t actually ask the person to do anything. And if you’re not going to ask them to do anything, then why leave a comment at all? I think it’s important to actually make it apparent that this is something you’re flagging and would like to see fixed in order to approve their PR. Not to mention, doesn’t this just make “This will improve performance and readability” the part that’s passing off an opinion as fact?
“Could” is such a meaningless, wishy-washy word here. Of course it could - are you telling me you think it’s better? Are you just throwing out a random option with some documentation you want me to read, while not being totally invested anyway, and you want me to defend what I’ve done?
It feels a bit condescending if anything, like “oh, I guess I just wasn’t informed enough” or else I would have known about this obviously better approach.
But anyway, if the author here takes issue with the idea of stating opinions as facts, then perhaps a better way is a simple:
At other times, it’s appropriate to not make a direct request, perhaps when you’re somewhat junior or you’re not positive yourself and just brainstorming a suggestion. But then, contrary to the article’s version, you should make it explicit that it’s a suggestion and you’re soliciting feedback:
I like to think of it in terms of “emotional labor”. Basically, make it easy on the author to respond to your PR, and don’t waste their time. If you feel strongly that your way is better, make the explicit request and the person can simply implement that change without thinking about it too much, even if they’re personally on the fence about whether it’s actually better. If you’re really not sure, leave it a question or give them an easy out, so the person can choose to not make your change quickly. But don’t leave it ambiguous about how invested you are in the comment. “This could have been done this way” is not super useful, less so with a bunch of documentation to read. Now the author is in the position of trying to infer how invested you are in the comment, whether you actually are asking for a change, or asking a question about their approach vs yours, and whether you should try to defend yours or what. There’s rarely an unequivocally “best” approach, and so an informational request-less comment invites the person to try to re-litigate for themselves the pros and cons of two different approaches they likely already considered.
Github’s “suggest” feature where you can simply suggest a fix for a number of lines and the author can commit that change from the web UI can save you from trying to phrase your change suggestion in words and you have an actual snippet of code to discuss further.
The software world is full of gordon ramseys and ramsey is a mirco managing fraud.
I don’t follow the connection between your comment and the post. Would you please explain?
Sure. Have you seen kitchen nightmares ? Toxic reviewers are like that.