3 ms·

The review experience can really vary between teams, and depending on whether you have readability or not and whether you're changing code you own or not. This

The review experience can really vary between teams, and depending on whether you have readability or not and whether you're changing code you own or not. This can make the difference between a couple of hours to a week to submit the same change. Readability in general is a process that while valuable, but can be really streamlined. I was working in a fast moving team using Python where everyone had readability, and I just didn't have time to delay most non trivial CLs, so even after submitting hundreds of CLs I did not have readability, having sent only a handful for review. Another negative effect I saw was people opting not to use a less popular language that was the right choice for a project, just because not many people in their office have readability in that language. A much easier process would go something like this: Suppose my teammate has readability in language X, They can tag review comments as being readability related or not. Once someone has N CLs under their belt with readability related comments under some low percentage in the past M CLs, they are granted readability automatically, or have to submit just 1-2 CLs for a speedy readability process.

dilyevsky 6y ago
Yeah python and c++ readability there is full on bs. No two readability reviewers agree on things, nitpicking was ridiculous and very superficial ime and most of those issues can be solved with some light linting/static analysis. Then you watch all those l6s+ shipping obviously terribly styled code bc noone dares to say shit... fwiw Go had much better process for this but i got mine when the readibility list was like a hundred folks so not sure about now
- slumpt_ 6y ago
  Google has heaps of horrible code. The problem with lots of small isolated CLs and little time invested in revisiting broader architectures. Then they’ll finally rewrite the thing some day, after a massive and often unnecessary design doc filled with needless bike-shedding that ultimately proposes an overengineered solution to help the person make L+1.
  - Goog_l_Dude_l 6y ago
    The things that always blows my mind is how every individual piece of code you read has good, clean, well documented, well tested code. But overall it's just a big mess. Not everything, though. Infrastructure code and libraries are usually very high quality and very nice to work with.
    - slumpt_ 6y ago
      That first paragraph resonates 200%. As long as you’re only looking at like 30 lines it’s lovely. Then you zoom out to view the beast.
jrochkind1 6y ago
I'm realizing you are using "have readability" in a specialized way I may not be following. Can you say more what you mean by "whether you have readability" and "readability is a process...". Readability is a process (not an adjective)? That only some teams have? Can you say more?
- Rebelgecko 6y ago
  thedance provided a nice overview of the "readability" process here: https://news.ycombinator.com/item?id=22620455 https://news.ycombinator.com/item?id=22620455 Essentially what it means is that, when writing code in certain programming languages, one of your code reviewers has to have proved that they're fluent in that language and the style guide for it.
- hemogloben 6y ago
  Based on [1]; readability in google means you've been certified to review code according to Google's code style in a given language. Seems like if you've been certified you don't need an extra person who has been certified in 'readability' to review your code so it makes it easier. 1) https://www.pullrequest.com/blog/google-code-review-readability-certification/ https://www.pullrequest.com/blog/google-code-review-readabil...
- joshuamorton 6y ago
  Code changes generally require 3 things: 1. LGTM (that is, any second set of eyes) 2. Readability Approval 3. Owners approval (similar to github ownership) The CL author can provide readability and ownership, if I have python readability and own the file I'm changing, anyone else can LGTM. But if not, at least one of my reviewers needs to provide the language approvals. There's a process for gaining readability in a particular language, where you send your normal code changes to be additionally reviewed by experts who give deeper feedback on language idioms/use. This process is contentious because Google is fairly strict about style guides, and the readability reviewers goal is to ensure that you are writing idiomatic code, not to ensure your changes get submitted fast, so they'll often request changes and delay things (but this is known in advance, and for a high priority change, there are other avenues to get language approvals). After you are granted readability, you no longer need approval from someone else with readability for changes in that language. Personally, I found the process valuable for all of the languages I've gone through it in, but I can absolutely understand people's frustration, especially when dealing with deadlines or similar.

dilyevsky 6y ago

Yeah python and c++ readability there is full on bs. No two readability reviewers agree on things, nitpicking was ridiculous and very superficial ime and most of those issues can be solved with some light linting/static analysis. Then you watch all those l6s+ shipping obviously terribly styled code bc noone dares to say shit... fwiw Go had much better process for this but i got mine when the readibility list was like a hundred folks so not sure about now

slumpt_ 6y ago
Google has heaps of horrible code. The problem with lots of small isolated CLs and little time invested in revisiting broader architectures. Then they’ll finally rewrite the thing some day, after a massive and often unnecessary design doc filled with needless bike-shedding that ultimately proposes an overengineered solution to help the person make L+1.
- Goog_l_Dude_l 6y ago
  The things that always blows my mind is how every individual piece of code you read has good, clean, well documented, well tested code. But overall it's just a big mess. Not everything, though. Infrastructure code and libraries are usually very high quality and very nice to work with.
  - slumpt_ 6y ago
    That first paragraph resonates 200%. As long as you’re only looking at like 30 lines it’s lovely. Then you zoom out to view the beast.

jrochkind1 6y ago

I'm realizing you are using "have readability" in a specialized way I may not be following. Can you say more what you mean by "whether you have readability" and "readability is a process...". Readability is a process (not an adjective)? That only some teams have? Can you say more?

Rebelgecko 6y ago
thedance provided a nice overview of the "readability" process here: https://news.ycombinator.com/item?id=22620455 https://news.ycombinator.com/item?id=22620455 Essentially what it means is that, when writing code in certain programming languages, one of your code reviewers has to have proved that they're fluent in that language and the style guide for it.
hemogloben 6y ago
Based on [1]; readability in google means you've been certified to review code according to Google's code style in a given language. Seems like if you've been certified you don't need an extra person who has been certified in 'readability' to review your code so it makes it easier. 1) https://www.pullrequest.com/blog/google-code-review-readability-certification/ https://www.pullrequest.com/blog/google-code-review-readabil...
joshuamorton 6y ago
Code changes generally require 3 things: 1. LGTM (that is, any second set of eyes) 2. Readability Approval 3. Owners approval (similar to github ownership) The CL author can provide readability and ownership, if I have python readability and own the file I'm changing, anyone else can LGTM. But if not, at least one of my reviewers needs to provide the language approvals. There's a process for gaining readability in a particular language, where you send your normal code changes to be additionally reviewed by experts who give deeper feedback on language idioms/use. This process is contentious because Google is fairly strict about style guides, and the readability reviewers goal is to ensure that you are writing idiomatic code, not to ensure your changes get submitted fast, so they'll often request changes and delay things (but this is known in advance, and for a high priority change, there are other avenues to get language approvals). After you are granted readability, you no longer need approval from someone else with readability for changes in that language. Personally, I found the process valuable for all of the languages I've gone through it in, but I can absolutely understand people's frustration, especially when dealing with deadlines or similar.