[HN Gopher] Improving code review time at Meta
___________________________________________________________________
Improving code review time at Meta
Author : pzrsa
Score : 17 points
Date : 2022-11-17 22:03 UTC (56 minutes ago)
(HTM) web link (engineering.fb.com)
(TXT) w3m dump (engineering.fb.com)
| Scubabear68 wrote:
| I came to the conclusion long ago that mandatory code reviews are
| a waste of time. For critical stuff, absolutely.
|
| But PRs and review cycles over burden dev teams and don't seem to
| move the quality needle higher one bit.
|
| A better way is to ensure multiple hands touch a given area of
| the code, so that multiple eyes ultimately are seeing and
| manipulating those bits. If they are given a task to do in that
| area they will be motivated to understand it (and potentially
| improve it). By contrast, with code reviews often the reviewer
| does not have time to really deep dive into the code and will
| only have a superficial understanding of it.
|
| Oh, also use code quality scanners to keep an eye on tactical
| code debt.
| posharma wrote:
| Disclaimer: these are anecdotal reports. I've heard from a lot of
| my friends how abysmal the quality of code is at Meta. Obviously,
| this may not be true in all teams/products, but that's the
| general sentiment. Why make it faster when you're already dealing
| with mess! This is abundantly evident from the constant fire
| fighting, duct tapes and a metric driven culture that
| incentivizes the number of diffs landed.
| rat9988 wrote:
| Because making it slower won't improve anything.
| cheriot wrote:
| Are there companies with a reputation for code quality?
| no-dr-onboard wrote:
| GitLab and Netflix come to mind.
| sgeisenh wrote:
| Google places a pretty high emphasis on code quality and
| readability. It's not universally great, but it's a big part
| of the culture. You can catch a glimpse in their [style
| guides][1], [abseil totws][2] and [aips][3]. Almost every
| change is required to be reviewed for "readability" in
| addition to functionality. This can feel like a lot, but it
| leads to pretty consistent style across the codebase which
| makes it a lot easier to switch projects or read and
| understand an unfamiliar section of the code.
|
| [1]: https://google.github.io/styleguide/ [2]:
| https://abseil.io/tips/ [3]: https://google.aip.dev/
| comfypotato wrote:
| Am I reading this right? If the total median time in review is "a
| few hours", that means that people are dropping what they're
| doing to review the code. That can't really be a good code
| review? A context switch alone would be half of that time for me.
| kweingar wrote:
| I work at a company with a similar code review culture.
|
| I have about 2 blocks of meetings per day on average. I usually
| check my assigned code reviews in the morning and when I come
| back from the meetings. Once I'm done with those then I move on
| to my own work.
|
| If I have too much code review to the point that I can't get my
| own work done, I take my name off the reviewer list and it's
| reassigned.
|
| On most days I'm still able to get a good 2-3 hours of
| sustained focused work on my own project.
| shortstuffsushi wrote:
| If I read it right, it described there being a "code review
| team." Whether they're responsible for reviewing all code, or
| just for coming up with these sorts of practices was not clear.
| ETA: with that said, the "reviewer recommender" to me would
| imply it's people who work on the same codebase.
| bsaul wrote:
| there's so many low hanging fruits for improving the quality of
| diff viewing. The worst code reviews are often the ones where
| code get refactored, leading to piles of delete / create lines
| that are just code being moved or slightly renamed.
|
| One very simple approach would be better git integration with the
| IDE, helping build commit that make sense, where a set of changes
| could easily be commented by the author as they're performing the
| edits, then keep improving from there.
| Calvin02 wrote:
| I wonder if there are opportunities for using GitHub CoPilot like
| system for better linters rather than rules based linting rules.
|
| Anyone working on this?
___________________________________________________________________
(page generated 2022-11-17 23:00 UTC)