4 ms·
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
by OneMoreGoogler 6y 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.
- cmrdporcupine 6y agoThat 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.
- ggm 6y ago2015 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?
- joshuamorton 6y ago> 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.
- OneMoreGoogler 6y agoIt 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.
- packetslave 6y agowell, 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.
- gct 6y agoAh yes, that's been rectified now =D
- B-Con 6y agoSounds 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.
- atombender 6y agoYour 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 https://news.ycombinator.com/item?id=18037184
- OneMoreGoogler 6y agoHa, busted.
- dandigangi 6y agoRekt
- grumple 6y agoMight be sensible not to directly out people in the future, they could change usernames for privacy reasons. Think of it like deadnaming someone.
- dandigangi 6y agoSelf merging your code at a company like Google is wild.
- throwaway53453 6y agoThat 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.
- jeffbee 6y agoYou 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-borg https://cloud.google.com/security/binary-authorization-for-b...