5 ms·

At Amazon, all code is reviewed. Comments are constructive and straight to the point. Standard courtesy applies, but anything more than that is left out. Unless

by amxpd 7y ago

At Amazon, all code is reviewed. Comments are constructive and straight to the point. Standard courtesy applies, but anything more than that is left out. Unless there's something extremely interesting, overly cautious phrasing and "positive" comments are mostly noise. Also, people tend to specify when comments are nits.

sanderjd 7y ago
Why the scare quotes around "positive"? I have lots of positive thoughts when doing code reviews. I don't write them into comments all that often, but sometimes I do. "Oh I didn't know about this API, nice find!" "Ah, nice approach, this is a big improvement." "Thanks for improving the test coverage!" Maybe this seems like unactionable "noise", but it isn't, it encourages future actions of the kind being positively reinforced. Plus it's free, it takes no time to write and no time to read comments like this, all it does is make the author's experience of reading the review a bit more pleasant. This is all upside, no downside. It would be crazy to discourage such a thing.
- aledalgrande 7y ago
  Agree, I love to point out a nice approach or a TIL, and I like that Google put it down in writing. If Amazon really considers it noise, it is very descriptive of their culture.
- UK-Al05 7y ago
  Depends how it's come across. If your not careful it can come across as patronising.
  - n_ 7y ago
    Full disclosure, I work at Google, and I actually find positive comments mostly rare, but code review culture is very different team to team, so I can hardly speak for the entire company. Personally I try to make them often, but I also try to be thoughtful about making them as to not come off patronizing like you mentioned. Though I’ve never felt like a positive comment ever made me feel patronized in any situation, I think a good way to not come off patronizing is to to make the comment more personal than general, ie: “This is a cool way to use the rest operator (or whatever thing), Ima steal this”. Obviously it needs to be an authentic comment, but I think it’s a significant difference vs “Great use of the rest operator!” When the interpretation on the other end might be “oh so this person is surprised I know these basic language constructs”. I learn things from doing code reviews all the time, and I like letting my coworkers know that I feel like I’ve gotten better at my job as a result of reviewing their contributions. I think it’d be a shame if I didn’t let them know.
  - afarrell 7y ago
    > it can come across as patronising Sometimes, it is worth the risk of sounding patronising.
aspaceman 7y ago
People call positive comments noise? They put scare quotes around “positive”? This field really scares me sometimes. Communicating positive feedback is crucial for teamwork, teaching, and passing ideas on.
- kortilla 7y ago
  Depends on level of seniority and amount of time the person has been on the team. Positive comments to a senior engineer about using a hash map instead of doing a huge linear scan are just noise. Nurses don’t compliment each other for using a new pair of gloves on each patient.
decebalus1 7y ago
If this is representative of the Amazon engineering culture, I'm gonna add this to the (already overflowing) pile of reasons I would never work there. Feeling valued is linked to job performance. And if I notice any of a) a generally interesting piece of code b) the engineer being a boy scout and fixing something in that particular module to make his changelist better c) a novel/comprehensive way of testing the code automatically d) elbow grease to just go the extra mile in terms of doing a great job (without adding unnecessary complexity) I will call it out in the code review along with the regular 'fix this' feedback. As a principal engineer my word carries some amount of weight and I truly want the person to feel good about their work when they deserve it. Another thing I often do is add a 'thank you for taking the time to do this' whenever someone slightly decreases the amount of tech debt (by refactoring, or removing code/complexity) when it was obvious it wasn't absolutely necessary to get their work item completed. Basically whenever someone shows they're thinking strategically rather than tactical about their work, I want to make sure that person knows that I noticed and that I value that. People aren't robots, we're all professionals and we all like to feel good about our work.
jsolson 7y ago
> Unless there's something extremely interesting, overly cautious phrasing and "positive" comments are mostly noise. Not really. Positive comments help teach engineers which things they've done that conform to local best practices (and why) without them having to meticulously dig those up (assuming they're even documented). A lack of positive comments leaves engineers to learn them only by running afoul of them. Effectively it provides direction only when some threshold of badness is crossed, while leaving positive comments on good code (especially for new team members or junior engineers) provides a beacon pointing away from the badness threshold entirely. Put differently: commenting on good code makes for swifter and less eventful code reviews by steering engineers away from the bad practices that make code challenging to review in the first place. Also, it's just nicer to spend forty hours a week with people who demonstrably appreciate their peers' good work.
- mytailorisrich 7y ago
  A code review is not the time and place to report positive or convoluted comments. A code review is to find issues and to report them clearly. The only positive thing to report should thus be that all is fine if the reviewer did not find any issues. Now, if you are conducting the review in a meeting then obviously you can make oral comments in passing. If you're using a software tool for reviews then the all the comments should be on point. Nothing prevent you from talking to your coworker afterwards to spread some love if you want to.
  - afarrell 7y ago
    > A code review is not the time and place to report positive ... comments. Why not? (Convoluted comments should be made more concise)
    - mytailorisrich 7y ago
      Because a code review is not to pat each other on the back, it is to inspect and report issues. It is already costly enough without going off topic. As said, if you want to praise then you are free to do it offline. This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years.

by amxpd 7y ago

sanderjd 7y ago

Why the scare quotes around "positive"? I have lots of positive thoughts when doing code reviews. I don't write them into comments all that often, but sometimes I do. "Oh I didn't know about this API, nice find!" "Ah, nice approach, this is a big improvement." "Thanks for improving the test coverage!" Maybe this seems like unactionable "noise", but it isn't, it encourages future actions of the kind being positively reinforced. Plus it's free, it takes no time to write and no time to read comments like this, all it does is make the author's experience of reading the review a bit more pleasant. This is all upside, no downside. It would be crazy to discourage such a thing.

aledalgrande 7y ago
Agree, I love to point out a nice approach or a TIL, and I like that Google put it down in writing. If Amazon really considers it noise, it is very descriptive of their culture.
UK-Al05 7y ago
Depends how it's come across. If your not careful it can come across as patronising.
- n_ 7y ago
  Full disclosure, I work at Google, and I actually find positive comments mostly rare, but code review culture is very different team to team, so I can hardly speak for the entire company. Personally I try to make them often, but I also try to be thoughtful about making them as to not come off patronizing like you mentioned. Though I’ve never felt like a positive comment ever made me feel patronized in any situation, I think a good way to not come off patronizing is to to make the comment more personal than general, ie: “This is a cool way to use the rest operator (or whatever thing), Ima steal this”. Obviously it needs to be an authentic comment, but I think it’s a significant difference vs “Great use of the rest operator!” When the interpretation on the other end might be “oh so this person is surprised I know these basic language constructs”. I learn things from doing code reviews all the time, and I like letting my coworkers know that I feel like I’ve gotten better at my job as a result of reviewing their contributions. I think it’d be a shame if I didn’t let them know.
- afarrell 7y ago
  > it can come across as patronising Sometimes, it is worth the risk of sounding patronising.

aspaceman 7y ago

People call positive comments noise? They put scare quotes around “positive”? This field really scares me sometimes. Communicating positive feedback is crucial for teamwork, teaching, and passing ideas on.

kortilla 7y ago
Depends on level of seniority and amount of time the person has been on the team. Positive comments to a senior engineer about using a hash map instead of doing a huge linear scan are just noise. Nurses don’t compliment each other for using a new pair of gloves on each patient.

decebalus1 7y ago

If this is representative of the Amazon engineering culture, I'm gonna add this to the (already overflowing) pile of reasons I would never work there. Feeling valued is linked to job performance. And if I notice any of a) a generally interesting piece of code b) the engineer being a boy scout and fixing something in that particular module to make his changelist better c) a novel/comprehensive way of testing the code automatically d) elbow grease to just go the extra mile in terms of doing a great job (without adding unnecessary complexity) I will call it out in the code review along with the regular 'fix this' feedback. As a principal engineer my word carries some amount of weight and I truly want the person to feel good about their work when they deserve it. Another thing I often do is add a 'thank you for taking the time to do this' whenever someone slightly decreases the amount of tech debt (by refactoring, or removing code/complexity) when it was obvious it wasn't absolutely necessary to get their work item completed. Basically whenever someone shows they're thinking strategically rather than tactical about their work, I want to make sure that person knows that I noticed and that I value that. People aren't robots, we're all professionals and we all like to feel good about our work.

jsolson 7y ago

> Unless there's something extremely interesting, overly cautious phrasing and "positive" comments are mostly noise. Not really. Positive comments help teach engineers which things they've done that conform to local best practices (and why) without them having to meticulously dig those up (assuming they're even documented). A lack of positive comments leaves engineers to learn them only by running afoul of them. Effectively it provides direction only when some threshold of badness is crossed, while leaving positive comments on good code (especially for new team members or junior engineers) provides a beacon pointing away from the badness threshold entirely. Put differently: commenting on good code makes for swifter and less eventful code reviews by steering engineers away from the bad practices that make code challenging to review in the first place. Also, it's just nicer to spend forty hours a week with people who demonstrably appreciate their peers' good work.

mytailorisrich 7y ago
A code review is not the time and place to report positive or convoluted comments. A code review is to find issues and to report them clearly. The only positive thing to report should thus be that all is fine if the reviewer did not find any issues. Now, if you are conducting the review in a meeting then obviously you can make oral comments in passing. If you're using a software tool for reviews then the all the comments should be on point. Nothing prevent you from talking to your coworker afterwards to spread some love if you want to.
- afarrell 7y ago
  > A code review is not the time and place to report positive ... comments. Why not? (Convoluted comments should be made more concise)
  - mytailorisrich 7y ago
    Because a code review is not to pat each other on the back, it is to inspect and report issues. It is already costly enough without going off topic. As said, if you want to praise then you are free to do it offline. This is a professional procedure in a professional setting, not warm words from an encouraging teacher at school... I don't want to have to go through comments that do not add any value to the exercise of finding issues, and I have never seen people leave such comments in 20 years.