8

What is the best way to do a code review for a colleague's code, without coming out as a complete ass?

Comments
  • 3
    It is just a code review. What did you plan on saying that you are that afraid?
  • 4
    I think a good approach is to not just show WHAT is bad and WHY, but to show and discuss about better approaches and bring in some real life examples why these improvements actually help everyone in the team.

    A good example is readability:
    Writing big methods with complex instructions and long comments will make it harder for a future coder to understand what it does.

    Or cosistent formatting:
    It makes code look similar, independent of who in the team wrote it. It feels more familiar to everybody and thus more maintainable and simpler to understand.
  • 3
    Constructively
    Refer to any coding standards your company has.
    Provide examples where something could be better.
    Provide positive comments as well.
    Try and make it a conversation.
  • 3
    Phrase some things as questions.. it's a hard one
  • 3
    @Benutzername the way my managers have taken my code review is by nitpicking issues which didn't seem like issues, at times asking me to completely change the system. I'm not saying they were always wrong, but it left a bad taste in my mouth. Now, I don't wanna do the same, so I want advice on how to actually approach it.
  • 1
    @Crost can you give some examples of this?
  • 4
    @LOLjustCoding if we rewrite this as an enum would we be able to get rid of these magic strings?
  • 9
    That said in my experience people don't want feedback, no matter what you do they will either get pissed off or ignore you. So I lean into it. I once left 120 comments.
  • 3
    @LOLjustCoding "nitpicking issues that didn't seem like issues" to you. Maybe that's a difference of opinion, but bear that in mind and try to explain why you think something is an issue. Coding standards are really good here, when you can just point to a document that says "do it this way".
  • 2
    But at about 30 comments, I start to feel bad...
  • 2
    LOL. You are just coding, man

    Keep your comments coming as long as they make a good difference. It is up to them to consider working on those comments
  • 5
    @asgs I dunno. Pre covid, I'd sit down with them and go through everything, particularly with junior devs. Now it's just "You've received an email: some bastard has left 50 comments on your code", feels a bit abstract and dehumanising.
  • 1
    @atheist totally agreed except you are not a bastard

    Zooming helps? We got to do what we have to do
  • 1
    what the others said is the best way:

    just be constructive and link any references you used to support your arguments.

    in cases where you're voicing a subjective opinion (like code style) you should check how it's done on other projects in your company, so it's consistent. In case there's no precedence to what you're discussing you need to explain what your reasons are for the change to be made.

    In subjective cases, be open minded too, the reviewee might come up with better reasons to keep his style or solution and you have to stay objective about how important the issue really is.

    especially with tiny nitpicks I like to message the guy saying something like" hey, I didn't find a lot of major issues, mostly just tiny points or questions, I'll leave the decisions up to you in case you disagree" or something similar.

    Important to remember that your job as a reviewer is to hold your teammate's back, you're there to spot bugs & mistakes early and that helps the entire team.
  • 1
  • 0
    uh... code reviews are usually very neutral as far as feedback goes, but a general rule for feedbacks: be specific and clear, don't spare words if it might compromise their understanding
Add Comment