Hey, I want your opinion on code reviews, what is the best way to use them in a professional environment? Pick one of the following and give me your thoughts (from the most forgiving to the most strict):

  1. no code reviews, they are useless
  2. optional code reviews
  3. mandatory reviews on code that is already merged, optional fixes
  4. mandatory reviews on code before merging (like a pull request), with a time-frame for optional fixes (i.e. whether to fix what has been pointed out is up to the author), merge will occur anyway.
  5. mandatory reviews on code before merging (PR) with mandatory fixes.

Of course in open source development with public contributions, you’ll often see (5), but I’m not convinced it could work in professional dev.

Edit: I’m talking about a team of 5 mid to senior devs (no junior or interns) working on a 2-3 year project without many security concerns, but feel free to give me your general opinion.

  • corsicanguppy@lemmy.caEnglish
    1·
    23 days ago

    you’re moving the problem to the reviewer,

    Nope. It’s not zero-sum. Coder and reviewer share the problem and it gets that extra set of eyes.

    what change request would you say is a mandatory fix?

    Errors have crept in and Prod is Down for someone (us or a customer) and a merge to a hotfix branch needs to go to Build right now.

    I have seen hot fixes that get a mob of reviewers, only because the fix needs to not introduce new glitches. It’s an all-hands situation.

    No, that’s not normal. If your fix can only be tested in the field by the customer - because you don’t have a boat engine big enough to be managed by your OS; fun story - then it’ll get all the scrutiny it can.