this post was submitted on 01 Sep 2023
336 points (96.2% liked)

Programming

17540 readers
68 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities [email protected]



founded 2 years ago
MODERATORS
you are viewing a single comment's thread
view the rest of the comments
[–] [email protected] 17 points 1 year ago (2 children)

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

[–] [email protected] 11 points 1 year ago (1 children)

I work at a small organization where code reviews are good, but I've noticed that the larger the code review, the faster it needs to get done in order to avoid merge conflicts, which means large code reviews are much less effective in proportion to the size.

[–] [email protected] 7 points 1 year ago

I’ve noticed that the larger the code review, the faster it needs to get done in order to avoid merge conflicts, which means large code reviews are much less effective in proportion to the size.

Exactly. And in larger corporations where you have many people contributing and the code is moving fast, having people nitpick your PRs just for show is crap because it delays everything sometimes by days. I had to say no very clearly to some people on code reviews because they were demanding me to place variables in alphabetical order on hard PRs that took quite sometime to get working and were very prone to code conflicts.

[–] [email protected] 11 points 1 year ago (2 children)

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.

[–] [email protected] 2 points 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.

[–] [email protected] 2 points 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