• lysdexic@programming.dev
    link
    fedilink
    English
    arrow-up
    11
    ·
    1 year ago

    Here’s another: most code reviews on larger companies are BS, just for show and nitpicking.

    Story time.

    Once I worked with a developer that just joined the company straight out of college, and had far more ambition than competency. That developer decided that code reviews where the venue where their high bar for code quality would shine, so they decided to nitpick everything that went against their poorly formed sense of taste. As luck would have it, the developer was assigned to a legacy project that was in cold storage for years and had no tests and linters, and was in a really poor state, and proceeded to leverage that to challenge each and every insignificant detail such as if a space should be at the left or at the right of a symbol. Each code review automatically received dozens of comments nitpicking whitespace changes. What a waste of time with so much noise.

    I drop by the project, and noticed the churn that developer alone forced upon everyone, as that team had a rule that all code reviews should be passed by all reviewers and that reviewer made it their point to reject reviews that didn’t complied to their opinion on whitespace. So the first thing I did was onboard a code formatter, and made it my point to subject the spec to an unanimous code review. That problematic developer made it their point to nitpick away each and every single setting, but it turned out some of their opinions conflicted with previous feedback.

    So the code formatter tool was onboarded onto the team. Did that stopped the problematic developer from continuing their nitpicking? No. Except this time around other developers started pushing back because the opinions were contradictory and contrasted with the official code formatting style.

    All it took was a couple of days for the problematic developer to go from dozens of comments per day to zero. The code formatter was still optional and not fully adopted, but the problematic developer simply ceased with the bickering.

    • StudioLE@programming.dev
      link
      fedilink
      arrow-up
      2
      ·
      1 year ago

      I wish this had been my experience. I pushed for so long in my last company for standards to be written, code formatters implemented and objectivity to be brought to reviews but it was always ignored.

      Instead I had to endure every employee who claimed seniority (in a non hierarchical company) subjecting their opinion on style in reviews. It came up the point that I dreaded having to work with specific people because they kept triggering my PTSD with their moving target of micro management.

      Only afterwards did I truly appreciate how poor a lot of their opinions were. Now one of my first questions when approaching a new project is what standards we’re following. If they look at me blank faced that’s a pretty solid red flag.

    • muddi [he/him]@hexbear.net
      link
      fedilink
      English
      arrow-up
      2
      ·
      1 year ago

      The circular reasoning I got after proposing to use a code formatter:

      • Why are you nitpicking on PRs? Let’s use a linter instead
      • Yes we already have a linter for compliance sake
      • Oh it’s turned off though. I don’t like it.
      • No we can’t use it actually, it’s a third party one and it’s not compliant.
      • We can’t use the first party one. It’s not extensive enough.
      • No we shouldn’t extend it with our own custom rules. It’s too much maintenance.
      • I refuse to use any IDE formatting shortcuts or plugins, and will commit my code as I feel. The problem is not how I write my code.
      • Why are you still discussing this? Didn’t you figure out how to use a linter yet?

      We’re still at square one with this after a year or so