[HN Gopher] Feedback Ladders: How We Encode Code Reviews at Netlify
___________________________________________________________________
Feedback Ladders: How We Encode Code Reviews at Netlify
Author : saadatq
Score : 41 points
Date : 2021-03-02 20:05 UTC (2 hours ago)
(HTM) web link (www.netlify.com)
(TXT) w3m dump (www.netlify.com)
| imwillofficial wrote:
| 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!
| leipert wrote:
| 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
| swyx wrote:
| 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.
| wbc wrote:
| Thanks for the write up!
|
| What are you thoughts on enforcing future action (pebbles)? One
| thing I try to push for is to never have TODOs in code, instead
| make a backlog ticket/issue and link it. Unfortunately those
| tend to stay de-prioritized.
|
| How often do your pebbles get the desired follow-up?
| harveywi wrote:
| 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.
| evantahler wrote:
| This is a really good and simple to use suggestion!
| PragmaticPulp wrote:
| 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?
| mattchamb wrote:
| 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.
| onion2k wrote:
| 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.
| EatsIndigo wrote:
| Tried it, didn't catch on because emojis are too far out of reach
| from the github UI lol
| swyx wrote:
| we dont actually use emojis - we just use names, eg. "[dust] i
| dont like how this looks but whatever", "[boulder] this code
| doesnt run"
| qbasic_forever wrote:
| 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.
| twelfthnight wrote:
| 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.
| PragmaticPulp wrote:
| 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.
| munchbunny wrote:
| 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.
| swyx wrote:
| 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.
| qbasic_forever wrote:
| 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.
___________________________________________________________________
(page generated 2021-03-02 23:01 UTC)