5 ms·

"Readability" works terribly when your company is acquired and your team enters all at the same time. Google has (or had ~10 years ago), a thing called "readab

"Readability" works terribly when your company is acquired and your team enters all at the same time. Google has (or had ~10 years ago), a thing called "readability" for each language, where in order to be allowed to commit code to the central "google3" repo, you needed to have written some large amount of code in that language, and needed to have a readability reviewer sign off on your code. The process is designed for slowly on-boarding junior people into a team, and introducing them to google coding style and practices. Eg, the senior, mentoring folks on the team do the reviews and bring the new person up to speed. I imagine it must work well in that context. However, this breaks down when your entire team is new. How do you find somebody to review the code? All several million lines of the product that was acquired? Especially when it is written in multiple languages. So we were basically locked out of the main corporate repo, unable to do anything productive. We finally figured out that there was a paved path with a git repo used by the kernel team (and android?) that had none of these hurdles, where we could put our code and get productive immediately.

jefftk 3y ago
The way it's supposed to work is that acquired teams get lots of support on integration, including readability. This helps your team get integrated into writing Google-style code. Not sure why that didn't work out in this case? (Left Google a year ago)
- badlucklottery 3y ago
  This. Someone dropped the ball. There's a form to get your corner of the codebase exempted from readability temporarily. This gives your team a quarter or two to build up readability.
  - Espressosaurus 3y ago
    A quarter or two isn't going to be enough for a drastic realignment of a large codebase. It's a start, but only a start.
    - summerlight 3y ago
      Readability is usually applied in an incremental way. You don't have to fix all the code to make it conformant. If there are concerns about consistency, the style guide actually encourages people to prefer consistency over its own rule.
    - badlucklottery 3y ago
      The 1-2 quarters is just to "realign" the SWEs, not the codebase. Old code can usually be left as-is unless there some particularly egregious security hole or the like.
filoleg 3y ago
"Readability" requirement is still a thing, but it isn't for every single piece of code in G3, and I haven't worked close enough to it to think about the exact mechanism of how it applies. My previous team - pretty much any python submission was hitting me with a python "readability" requirement, and it was a bit painful, because only a single person in my entire group of teams (roughly 15 people total) had the "python readability expert" status. My current team - already submitted quite a few significant C++/TS/Java pieces of code to G3, and not a single "readability" requirement triggered.
MH15 3y ago
"Readability" is very much still a thing. It's a mess and would be one of the worst things to take from Google. If you can't enforce the code style you like through autoformatters and linting, it's not worth enforcing.
- opportune 3y ago
  I kind of disagree in the sense that readability indirectly forces someone who has been at Google for a while/ is more experienced to have to sign off on new people’s code. Without it, you could have some very junior members with OWNERS reviewing other very junior members’ code. And there is more to style than just linting, IMO. For example in C++ there are some complex macro-based test predicates that are hard to learn and use but which greatly simplify/improve on naive testing. Part of the point of C++ readability is that people who understand this stuff teach new people how to use them, or at least introduce them to concept, during code review
  - greiskul 3y ago
    > I kind of disagree in the sense that readability indirectly forces someone who has been at Google for a while/ is more experienced to have to sign off on new people’s code. Without it, you could have some very junior members with OWNERS reviewing other very junior members’ code. Exactly. It is very likely for a lot of junior engineers will be working with other junior engineers, and they will in fact have the most specific knowledge of the part of the project that they are implementing. And human nature makes it so people are afraid of being judged by their "superiors". Readability makes so that it breaks that barrier, guarantees that a more senior engineer will be involved, and will teach the ropes into writing readable, mantainable code to nooglers.
    - jrockway 3y ago
      I dunno, I am pretty sure I got Java readability the second month I was at Google and was already in the OWNERS file. I was a readability reviewer and most of the readability CLs were the first project a person worked on at Google, often rather unnecessary but redone strictly to meet readability requirements (largely new code, more than X lines, etc.). I would go back and forth for quite a while to turn 1000 careless lines of throwaway code into 50 lines that were actually good, but I basically had to grant readability after that one interaction, and it never felt great to me. The most hated readability process at Google was Go's process (at least in the early days; k8s is obviously not using it), but I think it was actually one of the best. It took me a long time to get Go readability, but after going through the process I feel like I'd write the same Go code as anyone on the Go team. When I look at people's open source projects I think to myself "don't they know that that Simply Isn't Done?" But of course they don't; Go readability can only be experienced, not explained. People didn't like that process, and I am sure I said nasty things about it at the time, but in retrospect I really like it.
aaomidi 3y ago
There’s now an explicit safeguard against that.

jefftk 3y ago

The way it's supposed to work is that acquired teams get lots of support on integration, including readability. This helps your team get integrated into writing Google-style code. Not sure why that didn't work out in this case? (Left Google a year ago)

badlucklottery 3y ago
This. Someone dropped the ball. There's a form to get your corner of the codebase exempted from readability temporarily. This gives your team a quarter or two to build up readability.
- Espressosaurus 3y ago
  A quarter or two isn't going to be enough for a drastic realignment of a large codebase. It's a start, but only a start.
  - summerlight 3y ago
    Readability is usually applied in an incremental way. You don't have to fix all the code to make it conformant. If there are concerns about consistency, the style guide actually encourages people to prefer consistency over its own rule.
  - badlucklottery 3y ago
    The 1-2 quarters is just to "realign" the SWEs, not the codebase. Old code can usually be left as-is unless there some particularly egregious security hole or the like.

filoleg 3y ago

"Readability" requirement is still a thing, but it isn't for every single piece of code in G3, and I haven't worked close enough to it to think about the exact mechanism of how it applies. My previous team - pretty much any python submission was hitting me with a python "readability" requirement, and it was a bit painful, because only a single person in my entire group of teams (roughly 15 people total) had the "python readability expert" status. My current team - already submitted quite a few significant C++/TS/Java pieces of code to G3, and not a single "readability" requirement triggered.

MH15 3y ago

"Readability" is very much still a thing. It's a mess and would be one of the worst things to take from Google. If you can't enforce the code style you like through autoformatters and linting, it's not worth enforcing.

opportune 3y ago
I kind of disagree in the sense that readability indirectly forces someone who has been at Google for a while/ is more experienced to have to sign off on new people’s code. Without it, you could have some very junior members with OWNERS reviewing other very junior members’ code. And there is more to style than just linting, IMO. For example in C++ there are some complex macro-based test predicates that are hard to learn and use but which greatly simplify/improve on naive testing. Part of the point of C++ readability is that people who understand this stuff teach new people how to use them, or at least introduce them to concept, during code review
- greiskul 3y ago
  > I kind of disagree in the sense that readability indirectly forces someone who has been at Google for a while/ is more experienced to have to sign off on new people’s code. Without it, you could have some very junior members with OWNERS reviewing other very junior members’ code. Exactly. It is very likely for a lot of junior engineers will be working with other junior engineers, and they will in fact have the most specific knowledge of the part of the project that they are implementing. And human nature makes it so people are afraid of being judged by their "superiors". Readability makes so that it breaks that barrier, guarantees that a more senior engineer will be involved, and will teach the ropes into writing readable, mantainable code to nooglers.
  - jrockway 3y ago
    I dunno, I am pretty sure I got Java readability the second month I was at Google and was already in the OWNERS file. I was a readability reviewer and most of the readability CLs were the first project a person worked on at Google, often rather unnecessary but redone strictly to meet readability requirements (largely new code, more than X lines, etc.). I would go back and forth for quite a while to turn 1000 careless lines of throwaway code into 50 lines that were actually good, but I basically had to grant readability after that one interaction, and it never felt great to me. The most hated readability process at Google was Go's process (at least in the early days; k8s is obviously not using it), but I think it was actually one of the best. It took me a long time to get Go readability, but after going through the process I feel like I'd write the same Go code as anyone on the Go team. When I look at people's open source projects I think to myself "don't they know that that Simply Isn't Done?" But of course they don't; Go readability can only be experienced, not explained. People didn't like that process, and I am sure I said nasty things about it at the time, but in retrospect I really like it.

aaomidi 3y ago

There’s now an explicit safeguard against that.