5 ms·

It seems I disagree w/ many points. > - Leave actionable comments > suggestion: This is not worded correctly. > Can we change this to match the wor

It seems I disagree w/ many points. > - Leave actionable comments > suggestion: This is not worded correctly. > Can we change this to match the wording of the marketing page? If I already know how it should be, I prefer to quote it right away and save work, instead of asking and leaving room for misinterpretation. Asking also seems dishonest if I'm not truly inviting a discussion. I would still add a rationale so others can correct me though ("Changing this to match the wording on the marketing page"). > - Combine similar comments Sometimes it's better to be repetitive in the name of exactness, because most review tools are line-oriented. Otherwise I have to write a comment "See lines A, B, C, D, E", and the person asking for the review now has more work to do searching all the instances. > - Replace “you” with “we” Better yet, I replace all pronouns w/ "this". Criticise the work, don't make it personal if it's not necessary. "We" feels forced team-building, and dishonest if I'm not actually going to put in work. It seems these practices are created because people are using reviews as some form of conversation, and then everybody is stepping on egg shells. I believe if the entire team agrees to see it as just annotations (it's not a performance evaluation, leave your ego at the door, etc) these problems are avoided. (I can see "why" you need these guidelines though, I guess it depends on what kind of work culture you have, company size, and so on.)

brailsafe 6y ago
Agreed. My thought was at least that the tone of the chosen rhetoric seemed sort of passive-aggressive or meek. I tend to prefer use of phrases like "suggestion: This seems like it would be improved by more closely resembling the other marketing pages". That said, asking a question is a communication tactic that is supposed to illicit a response, however disingenuous it may be.
- nine_k 6y ago
  Asking a question makes the reader find the answer, and thus understand it, and the reasons for it. Also, sometimes the answer won't be the one you'd expected.
  - brailsafe 6y ago
    Yes, but so does just stating something. If it's incorrect, someone will tell you. That's how this forum works, hence your answer ;)
nine_k 6y ago
Whatever you may think, people do see code reviews as conversations, and do take comments personally deep inside, even if they try hard to avoid this in the name of professionalism. Rather than forcing people to exercise self-discipline while reading, it's nicer to exercise it yourself, and write comments in an impersonal way. Assuming that the reader is as smart as yourself just follows logically from the idea of peer review. This is what I got from about 10 years of doing code reviews, which sometimes needed to be scathing by nature; to induce the desirable changes you've got to genuinely sound like you're acting in the author's best interests, not just do so.
rgoulter 6y ago
Yeah, I can agree with "this depends on the kind of work culture etc.". Asking also seems dishonest if I'm not truly inviting a discussion. I think it's really worth emphasising this. Further: If the reviewee understands that feedback can be discussed, then the reviewee can discuss whether they're asked or told. If the reviewer has more authority than reviewee, then asking really ought to be asking a question. ...then everybody is stepping on egg shells. I think the intention behind rephrasing from "this is wrong" to "can it be this instead?" is to try and take the ego out of it. Except people are complicated and 'taking the ego out of it' isn't just down to a question of phrasing. The reason code reviews are done is to try to catch mistakes earlier. (Mistakes are more expensive to once more development has been done on a task). I like the term "psychological safety". It strikes me as 'unsafe' if a review comments are a matter of the reviewer's ego over the reviewee's. It should be fine to make mistakes, and to have those mistakes corrected. Because people are complicated and emotional.
zerd 6y ago
It's very easy to seem rude in text comments, even if it was not meant that way at all. "Change 'this' to 'that'" is much harsher than "Could you change 'this' to 'that'?" It's a small change in wording, but it changes the interpretation from "This is wrong, do this instead, I know better than you" to "I think this is a better way to do it".
- luckylion 6y ago
  I feel like if this is an issue, there's a larger issue in the team. I've only ever witnessed language be an important issue if people felt overwhelmed, stressed and treated badly. On the other hand, when people feel treated fairly and are happy, most don't care for niceties and prefer straight to the point communication. Especially rewording change requests as questions is something I don't like. Is this something that should be done? Is it just a question about the technical possibility? A request for comments about the intended change? (I've seen them all and the more friendly they are worded, the harder they are to tell apart)
- hcarvalhoalves 6y ago
  Is it really? I can find instructions disguised as questions condescending, for example. I believe no amount of wording will help if people are not trusting each other. I would rather work on team building than comment guidelines.
  - brujoand 6y ago
    Yes and no. It depends on the team and the culture. My favorite was when I worked in a team of only senior engineers we would leave comments like 's/this/that/g other than than LGTM'. This was all that was needed, no fluff. In other teams, a comment like this would have meant a chat with HR. We can't make rules for these things, because people and thus teams are different.
  - Cthulhu_ 6y ago
    But instructions are given by superiors, while code reviews (generally) by peers. If someone you don't respect starts barking "Change 'this' to 'that'", would you? I mean sure, in some cases you can completely disassociate from the human side of code, disregard a comment's name in favor of objective truth or however you want to word it, but in a lot of instances you still have to share the same office with that person.
Cthulhu_ 6y ago
I haven't done code reviews in forever, but when I did them, sure there were some things that were plain wrong, but there were also a lot of things where I wasn't sure if it actually needed a change - a confirmation from the developer would be required. And then there's things that I would like to see changed, but as a personal preference; when it comes down to it, it doesn't make a difference and I don't really care, so I would posit it as a suggestion instead of a command. Mind you, if I didn't really care either way, I could've chosen to ignore it as well. But you know how some people get <_<

brailsafe 6y ago

Agreed. My thought was at least that the tone of the chosen rhetoric seemed sort of passive-aggressive or meek. I tend to prefer use of phrases like "suggestion: This seems like it would be improved by more closely resembling the other marketing pages". That said, asking a question is a communication tactic that is supposed to illicit a response, however disingenuous it may be.

nine_k 6y ago
Asking a question makes the reader find the answer, and thus understand it, and the reasons for it. Also, sometimes the answer won't be the one you'd expected.
- brailsafe 6y ago
  Yes, but so does just stating something. If it's incorrect, someone will tell you. That's how this forum works, hence your answer ;)

nine_k 6y ago

Whatever you may think, people do see code reviews as conversations, and do take comments personally deep inside, even if they try hard to avoid this in the name of professionalism. Rather than forcing people to exercise self-discipline while reading, it's nicer to exercise it yourself, and write comments in an impersonal way. Assuming that the reader is as smart as yourself just follows logically from the idea of peer review. This is what I got from about 10 years of doing code reviews, which sometimes needed to be scathing by nature; to induce the desirable changes you've got to genuinely sound like you're acting in the author's best interests, not just do so.

rgoulter 6y ago

Yeah, I can agree with "this depends on the kind of work culture etc.". Asking also seems dishonest if I'm not truly inviting a discussion. I think it's really worth emphasising this. Further: If the reviewee understands that feedback can be discussed, then the reviewee can discuss whether they're asked or told. If the reviewer has more authority than reviewee, then asking really ought to be asking a question. ...then everybody is stepping on egg shells. I think the intention behind rephrasing from "this is wrong" to "can it be this instead?" is to try and take the ego out of it. Except people are complicated and 'taking the ego out of it' isn't just down to a question of phrasing. The reason code reviews are done is to try to catch mistakes earlier. (Mistakes are more expensive to once more development has been done on a task). I like the term "psychological safety". It strikes me as 'unsafe' if a review comments are a matter of the reviewer's ego over the reviewee's. It should be fine to make mistakes, and to have those mistakes corrected. Because people are complicated and emotional.

zerd 6y ago

It's very easy to seem rude in text comments, even if it was not meant that way at all. "Change 'this' to 'that'" is much harsher than "Could you change 'this' to 'that'?" It's a small change in wording, but it changes the interpretation from "This is wrong, do this instead, I know better than you" to "I think this is a better way to do it".

luckylion 6y ago
I feel like if this is an issue, there's a larger issue in the team. I've only ever witnessed language be an important issue if people felt overwhelmed, stressed and treated badly. On the other hand, when people feel treated fairly and are happy, most don't care for niceties and prefer straight to the point communication. Especially rewording change requests as questions is something I don't like. Is this something that should be done? Is it just a question about the technical possibility? A request for comments about the intended change? (I've seen them all and the more friendly they are worded, the harder they are to tell apart)
hcarvalhoalves 6y ago
Is it really? I can find instructions disguised as questions condescending, for example. I believe no amount of wording will help if people are not trusting each other. I would rather work on team building than comment guidelines.
- brujoand 6y ago
  Yes and no. It depends on the team and the culture. My favorite was when I worked in a team of only senior engineers we would leave comments like 's/this/that/g other than than LGTM'. This was all that was needed, no fluff. In other teams, a comment like this would have meant a chat with HR. We can't make rules for these things, because people and thus teams are different.
- Cthulhu_ 6y ago
  But instructions are given by superiors, while code reviews (generally) by peers. If someone you don't respect starts barking "Change 'this' to 'that'", would you? I mean sure, in some cases you can completely disassociate from the human side of code, disregard a comment's name in favor of objective truth or however you want to word it, but in a lot of instances you still have to share the same office with that person.

Cthulhu_ 6y ago

I haven't done code reviews in forever, but when I did them, sure there were some things that were plain wrong, but there were also a lot of things where I wasn't sure if it actually needed a change - a confirmation from the developer would be required. And then there's things that I would like to see changed, but as a personal preference; when it comes down to it, it doesn't make a difference and I don't really care, so I would posit it as a suggestion instead of a command. Mind you, if I didn't really care either way, I could've chosen to ignore it as well. But you know how some people get <_<