4 ms·

Preface: I was also very glad to see this called out specifically, and think it's a great rule. That said... > Note how Google does NOT say "make sure the cod

by jonahx 7y ago

Preface: I was also very glad to see this called out specifically, and think it's a great rule. That said... > Note how Google does NOT say "make sure the code is properly architected". is not accurate. The very first paragraph on the same page is: > Design > The most important thing to cover in a review is the overall design of the CL. Do the interactions of various pieces of code in the CL make sense? Does this change belong in your codebase, or in a library? Does it integrate well with the rest of your system? Is now a good time to add this functionality? Emphasis mine. But that sure sounds like they care about proper architecture. They just also care about avoiding over-engineering. They're not mutually exclusive.

flowerlad 7y ago
But that sure sounds like they care about proper architecture. Of course they care about architecture—I don’t think anyone implied otherwise. But at good companies like Google good architecture is a given. Top developers often fall into the pit of over-engineering, and almost never under-architect. So at top companies code reviewers have to be vigilant about over-engineering and rarely have to worry about under-architecting.
- deepaksurti 7y ago
  So for anyone, including me, when looking out for the next job should be: - Is `good architecture` a given in this team? - What are they doing to avoid `over engineering`? What are the specific questions one can ask to find out the answers to the above two?
- kortilla 7y ago
  >But at good companies like Google good architecture is a given. Absolutely not. Google’s interview process and inflow of fresh graduates does not bode well for good architecture. Having spent time at G and FB, I can certainly tell you that employees at both are no better at architecting code in a sane way than SWEs at other companies. Code architecture requires experience. Google does not.
  - mav3rick 7y ago
    Most teams at Google have senior devs that ensure this. The average level is way higher than other companies.
  - chrisseaton 7y ago
    I think the hiring bar at places like Google and FB is astronomically higher than almost all other places - they definitely have generally higher skilled people there on average.
    - mianos 7y ago
      The process does not encourage experience but mainly the ability to remember recent academic programming puzzles. The first comment is correct even though they do have many existing experienced developers that would be in a position to provide architectural advice.
    - xthrow 7y ago
      Checking for memorized algorithms and an MIT degree do not guarantee good code. I've seen more horrible code at a FAANG company than in many other places. Complete absence of overflow analysis in C++, race conditions, architecture astronauts, exploding code bases that no one understands any more. Features are being added on a daily basis, basically what matters is a high line count.
      - mav3rick 7y ago
        You clearly haven't seen code at Google and its "base" library.
    - kortilla 7y ago
      The hiring bar at Google does not test for the ability to organize code coherently. The skill is completely unrelated to algorithms and data structures. It’s like requiring candidates to deadlift 500 lbs and then assuming it means they can all run marathons.
- deleted 7y ago
  [deleted]
jxramos 7y ago
what does the _CL_ notation indicate?
- scruple 7y ago
  Changelist, perhaps?
  - gopalakrishnans 7y ago
    Thanks. This would be a nice thing to explain in the document that's open sourced.
    - svat 7y ago
      The HN submission links directly to https://google.github.io/eng-practices/review/reviewer/ https://google.github.io/eng-practices/review/reviewer/ but if you start at the top namely https://google.github.io/eng-practices/ https://google.github.io/eng-practices/ then there's a Terminology section right there on the first page. Maybe the GitHub Pages theme used by this site should be changed to one in which where every page links to its parent page (right now there doesn't seem to be any way to navigate upwards except by editing the URL).
- tylerl 7y ago
  "changelist" is the term perforce uses for a a single commit before it gets committed. A bit like a PR in git. Google used perforce before rolling their own vcs, and they kept the perforce terminology.

by jonahx 7y ago

flowerlad 7y ago

But that sure sounds like they care about proper architecture. Of course they care about architecture—I don’t think anyone implied otherwise. But at good companies like Google good architecture is a given. Top developers often fall into the pit of over-engineering, and almost never under-architect. So at top companies code reviewers have to be vigilant about over-engineering and rarely have to worry about under-architecting.

deepaksurti 7y ago
So for anyone, including me, when looking out for the next job should be: - Is `good architecture` a given in this team? - What are they doing to avoid `over engineering`? What are the specific questions one can ask to find out the answers to the above two?
kortilla 7y ago
>But at good companies like Google good architecture is a given. Absolutely not. Google’s interview process and inflow of fresh graduates does not bode well for good architecture. Having spent time at G and FB, I can certainly tell you that employees at both are no better at architecting code in a sane way than SWEs at other companies. Code architecture requires experience. Google does not.
- mav3rick 7y ago
  Most teams at Google have senior devs that ensure this. The average level is way higher than other companies.
- chrisseaton 7y ago
  I think the hiring bar at places like Google and FB is astronomically higher than almost all other places - they definitely have generally higher skilled people there on average.
  - mianos 7y ago
    The process does not encourage experience but mainly the ability to remember recent academic programming puzzles. The first comment is correct even though they do have many existing experienced developers that would be in a position to provide architectural advice.
  - xthrow 7y ago
    Checking for memorized algorithms and an MIT degree do not guarantee good code. I've seen more horrible code at a FAANG company than in many other places. Complete absence of overflow analysis in C++, race conditions, architecture astronauts, exploding code bases that no one understands any more. Features are being added on a daily basis, basically what matters is a high line count.
    - mav3rick 7y ago
      You clearly haven't seen code at Google and its "base" library.
  - kortilla 7y ago
    The hiring bar at Google does not test for the ability to organize code coherently. The skill is completely unrelated to algorithms and data structures. It’s like requiring candidates to deadlift 500 lbs and then assuming it means they can all run marathons.
deleted 7y ago
[deleted]

jxramos 7y ago

what does the _CL_ notation indicate?

scruple 7y ago
Changelist, perhaps?
- gopalakrishnans 7y ago
  Thanks. This would be a nice thing to explain in the document that's open sourced.
  - svat 7y ago
    The HN submission links directly to https://google.github.io/eng-practices/review/reviewer/ https://google.github.io/eng-practices/review/reviewer/ but if you start at the top namely https://google.github.io/eng-practices/ https://google.github.io/eng-practices/ then there's a Terminology section right there on the first page. Maybe the GitHub Pages theme used by this site should be changed to one in which where every page links to its parent page (right now there doesn't seem to be any way to navigate upwards except by editing the URL).
tylerl 7y ago
"changelist" is the term perforce uses for a a single commit before it gets committed. A bit like a PR in git. Google used perforce before rolling their own vcs, and they kept the perforce terminology.