[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)