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.
It’s an interesting system. On caveat: the Rock size does not really encode the intention of the comment. Some of my colleagues took the time to write up Conventional Comments which they use, I really like that it distinguishes between blocking/non-blocking issues, questions and even praise.
- Site: https://conventionalcomments.org/
- Discussion: https://news.ycombinator.com/item?id=23009467
oh wow nice to see this on HN! I wrote this blogpost - happy to answer any questions!
yes it works very well for Netlify; when i joined the frontend dev team i immediately saw the value of this idea and begged the eng and design team members (that came up with the idea) for permission to write it up
for those interested, I recently adapted this idea for project prioritization https://twitter.com/swyx/status/1366849232959807489?s=20 because I was searching for a better alternative to the nonsensical effort estimation vs impact backlog grooming systems prevalent in product management.
I found this very helpful. As a geologist who just submitted His first commit at a major tech company, the review process was scary and confusing. This type of roadmap would have made it rock solid.
This is tortuous double speak.
I used to work in a 10k employee mega organization where we had this kind of over formalization. It was probably invented to prevent feelings being hurt.
I then moved to a tiny five man consultancy where efficiency was what we lived (or died) by. Never forget the day when my first code review from the CEO was "that is f##king s@#t code!
I like it. I've also taken to doing something similar with other interactions - when weighing in on decisions I try to say up front how much I care about the feedback I'm giving. Same with messaging people on slack - if something is non urgent or has no action required, I try to say that up front. No idea if this helps people on the other end of my messages, but I like to think it does.
Personally I prefer to just have two levels, blocking and non-blocking. Outside of that I should be able to trust the code author to appropriately respond to comments, I don't see why I as the reviewer should be the one deciding on the follow up action. I can see how it might be more valuable for more junior members of the team though.
I really like this terminology! I might see if my team could use this, I've found that not everyone interprets code review comments the same way.
I'm working on a new tool to vastly improve human code reviews for teams on GitHub. One of the main things is making sure that each conversation is "resolved" so that there is no disagreement on when a PR is ready to go.
If you're interested in better code review for your GitHub repo, email me at sam@habosa.com and I can show you what I'm working on. We're in a private Alpha but definitely ready for use by real teams in everyday reviews.
> One final tip for asynchronous code reviews: Roundtrips (from teammate to reviewer back to teammate) are expensive, costing up to two working days of a sprint each time.
Um, we're doing it wrong.
I found this very helpful. As a non-programmer who just submitted His first commit at a major tech company, the review process was scary and confusing. This type of roadmap would have made it understandable and clear.
Edit: The down vote was because it was YAML wasn’t it? I know, I know, it’s not real programming.. But it still felt like an accomplishment!
I've prioritized bugs in a similar way for years (P1 "This bug will cause data loss in prod" through to P4 "There's is a spelling mistake"), but it's never occurred to me to do the same sort of useful labelling of things in PRs. This is a great idea that I'm going to take to my team.
This seems to take the view that all feedback is pointing out shortcomings.
That's not the only type of feedback that's useful. Sometimes I might just want to ask a question, or propose another idea that may or may not be better than what was done. Sometimes I have testing suggestions. These are not problems.
Is there any metric to prove that this proposal works?
I mean, you can impose all sorts of rules and whatnot, but it's all [dust] until it improves a metric that the company cares about (e.g. review time, # of bugs, etc.).
I am not sure that I want to get reminded every code review that I am from a country without boulders or mountains
Tried it, didn't catch on because emojis are too far out of reach from the github UI lol
This is a 5 point rating system. Personally, I try to stick to as few points as possible.
The consumer of the rating (in this case, the code author) usually needs to know one thing above all else: Do I take action or not. A two-point system is the default.
IMO, in a code review, the feedback should be either expected action or not. "I recommend you change this" or "FYI, consider this", aka, "required" or "optional".
Ultimately, as the author, I want to know if I'm being told "please change this" or "just FYI". Details can be haggled if necessary.
The strength of the recommendation isn't relevant unless there's push back, in which case the details can be haggled for the situation at hand. But trying to do that bucketing up front sounds like extra work that's usually unnecessary. As long as the reviewer is pursuing productivity, they can adjust their recommendation as they learn more.
Jesus christ man, just learn to talk gud
Title should be updated to add ‘(2020)’.
This is a really good and simple to use suggestion!
Code review is a sticking point for a lot of the junior hires I've worked with. I now give them a talk about how code reviews are not to be taken personally, how they should assume best intent, and some guidance for how to handle disagreements.
In my experience, setting an expectation that teams need to be respectful and communicative in code reviews, combined with no tolerance for bad behavior, is sufficient to keep the code review process running smoothly.
I'm not a fan of explicit process documents that prescribe communication practices to employees. There is a very specific type of person who loves to be handed a rule book for how to communicate with others, and they love these systems. They prefer to operate within structured environments where they can read the rulebook ahead of time, or perhaps even write the rulebook for others to follow. Having the rules spelled out like this brings an artificial sense of comfort, especially to those who otherwise struggle with organic communication.
For everyone else, the complexity of these systems is unnecessary mental overhead. Do two developers who were already communicating just fine need to adopt this process? More importantly, does the process stop here, or will there be additional sets of communication rules whenever another possible communication ambiguity arises?