Hacker News new | ask | show | jobs
by qbasic_forever 1936 days ago
Now instead of just dimensions for style, clarity, function, performance to argue about in code review you have a new dimension of terminology to bike-shed too:

"I think this is a boulder of a problem."

"No, no, no this is clearly dust."

"Looks like a pebble to me!"

I get the idea but IMHO you really need from the start to have some way to measure and quantify if this is actually improving code reviews in your organization. Do you have metrics showing amount of time spent on reviews, amount of bugs generated by commits, etc. and are you tracking them going up or down?

I can also see a certain path of dysfunction where people get known by their perception of problems. Suddenly Joe doesn't get asked for many reviews because he's "the mountain guy" that always thinks things are blockers. There really needs to be some thought around calibration and review of people's prioritization. Just adding names and terminology doesn't solve that problem.

4 comments

I get the feeling this sort of solution is for a problem specific to a small number of devs. For example, "we have a new developer on the team and they don't know what to expect" or "we have an expert dev who only speaks in koans!". Instead of handling these on a case by case basis by team leads, a new requirement was made for all developers.
Agreed. This type of process feels obvious to those who implement it, but becomes a burden that everyone else has to bear forever.

As this type of process accumulates, it becomes more and more painful for newcomers trying to join the team without breaking the rules. Obviously one requirement about prefixing code review comments isn't much, but in my experience it usually doesn't stop here. People who like creating process and rules tend to want to create a lot of process and rules for everyone else to follow.

Bike-shedding doesn't need any specific process to happen, it just needs people who want to bike-shed. If it's not over the severity label on the feedback, it'll just take some other manifestation.

The solution isn't to try to design a process to prevent bike-shedding, because you can't. It's to practice a culture that frowns upon bike-shedding and focuses on the using time well.

> I can also see a certain path of dysfunction where people get known by their perception of problems. Suddenly Joe doesn't get asked for many reviews because he's "the mountain guy" that always thinks things are blockers.

Joe probably doesn't need to declare everything "mountain" to become the guy that makes mountains out of molehills, and probably doesn't need the formalism to be the guy people don't like to ask for code reviews.

> There really needs to be some thought around calibration and review of people's prioritization.

Usually this is where we'd expect the manager to talk to Joe about being more constructive in code reviews.

in an ideal world, yes it'd be nice to quantify. in practice, it would take extra dev time to quantify. time that we dont have. we have to rely on our engineers' subjective feelings about our engineering practices.

i understand the desire to be data driven. and your point about making everything a blocker. if you're at a point where you dont trust the judgment of your colleagues, you should drop everything and address that right away.

but, most of the time, operating in good faith works.

Be careful, this is how institutions bake-in bias. Subjective feelings are littered with implicit and explicit bias at every level.

At the end of the day it's all just people management problems. You're right it's a time sink for devs to quibble over and quantify prioritization--that's why I think it's more of a thing for management to assess and review. Trust devs to assess priority on their own, but verify with regular followup and review that those assessments are accurate. Business needs will change too and that might alter priorities--what was dust last week is now an enormous mountain because customer X demands the obscure feature no one cared about before.

Agree. the post is silly beyond words, sorry author. it feels like it was written for linkedin, not HN.

The world of open source have already settled for: "nit" or "nitpick" for things that you can live with without changing in a code review.

Everything else is just noise. Either you must change, or not. Anything in between is just harming communication.