Hacker News new | ask | show | jobs
by OneMoreGoogler 1905 days ago
I worked at Google circa 2015, and found the code review process to be actively terrible.

I was writing JS, while my sole teammate was a firmware engineer. He was not willing or able to meaningfully review my code, so I had to try to scrounge random reviewers from other teams. They naturally de-prioritized it, so it took forever. And because they were not involved in my project, they would only have local stylistic feedback.

Months in, I finally figured out that other engineers were just self-reviewing: you can add yourself as a reviewer and just click merge. So I started doing that too.

5 comments

That honestly sounds very atypical. I've been at Google 9 years and never experienced the scenario you describe. You should probably have reached outside of your team for assistance or flagged a manager.

Yes, there have been times when code review wasn't super useful, but on the whole I never experienced the kind of dysfunction you're talking about and merging reviews without external review is a pretty big no-no.

2015 was 6 years ago. Google as possibly changed? Doubled in size (or more) by staff? has more rigorous approaches? has more people capable of meaningfully reviewing code?
Your comment seems to be exactly the same as this one [1] from the original HN story in 2018, except with less detail. Are you ridiculous_fish, or is this just a huge coincidence or something that happens all the time at Google?

[1] https://news.ycombinator.com/item?id=18037184

Might be sensible not to directly out people in the future, they could change usernames for privacy reasons. Think of it like deadnaming someone.
Ha, busted.
Rekt
> Months in, I finally figured out that other engineers were just self-reviewing: you can add yourself as a reviewer and just click merge. So I started doing that too.

Where? This certainly isn't possible in google3 (and wasn't possible in 2015 either).

That said, there are now some changes made to make, in general, the code review process for people without readability and without teammates who have readability more streamlined.

It was in the Android repo, which used Gerrit.

I never dealt with readability, because Android did not require it. In fact that was part of their internal recruiting pitch: fed up with readability? Come work on Android! Heh.

well, Android was always a bit of a special snowflake compared to the rest of the company. Separate codebase, separate build system, separate SCCM, etc. You can't really compare an Android team to a google3 team.

Culturally, it was worse when Andy was in charge, but it was still true when I left in 2018.

Ah yes, that's been rectified now =D
Sounds like a team health issue. Sounds like a hard time, my sympathies.

In my N years of experience, very few engineers are in an environment where only they have meaningful context on what they're working on. Generally any non-trivial project worth staffing is worth having two headcount work in the space, even if one is a TL with divided attention.

Self merging your code at a company like Google is wild.
That does not sound like they were working in the main trunk. I do not believe you can run any production code that was self signed like this.
You can't. Borg binary authentication is integrated with build and review. A build that does not descend entirely from reviewed, approved, and committed code running as a production user with access to userdata will raise alerts. Individuals are able to run non-committed code on Borg under their own accounts, but not under production role accounts.

You can break glass in emergencies by committing code TBR, or "to be reviewed", however this automatically escalates to owners of the code in question plus your manager and director, and all TBRs have to be resolved by actual review within a short time. An author cannot submit to-be-approved code; they have to be owners of the code in question (personally or transitively included in the OWNERS file) to TBR.

You can read about this system here: https://cloud.google.com/security/binary-authorization-for-b...