Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

A study of Google's code review tooling (Critique)

engineercodex.substack.com

Post reply on HN
Print view View original

21–30 of 302 posts

Re: A study of Google's code review tooling (Critique)

#21

Post by dontreact »

jeffbee wrote: post #10
dontreact wrote: post #3

I generally like the tool. When your code review tool is really nice one unexpected negative is that it can create a culture of nitpicking. Sometimes it’s not worth arguing over small details like variable naming but the tool makes it really easy for things to head in that direction. Sometimes variable naming isn’t a small detail. But sometimes it is and it’s a waste of everyone’s time to argue about it.

Writing readable code is not "nitpicking". It is common that an author doesn't see why their parameter name or function name is misleading but their reviewer, who comes to the change without as many preconceptions, sees it right away.

Im not saying it’s always nitpicking. I’m saying that sometimes it is.

Re: A study of Google's code review tooling (Critique)

#22

Post by marcinzm »

jeffbee wrote: post #19
tallowen wrote: post #13

I’m always surprised to see the totality of support for this workflow. My biggest gripes were: - Owners - Readability review (or really the 18month queue to get Java/python readability) Although seemingly innocuous, this made maintaining internal libraries very challenging. There was no way to update every call site of your library in an efficient way. Tools like Rosie were added on top so that you could shard your P…

Google3 is a huge codebase of high quality that moves exceptionally quickly, so it just seems like your priors are getting in the way. Complaining that a large-scale change among tens of thousands of software developers requires a tool (Rosie) is sort of the same thing as all those people who complain that it's too hard to cope with having millions of machines in prod, i.e. the type of people who wash out of the comp…

The same applies to Facebook which is their comparison and not some random 10 person startup.

Re: A study of Google's code review tooling (Critique)

#23

Post by bheadmaster »

jeffbee wrote: post #10
dontreact wrote: post #3

I generally like the tool. When your code review tool is really nice one unexpected negative is that it can create a culture of nitpicking. Sometimes it’s not worth arguing over small details like variable naming but the tool makes it really easy for things to head in that direction. Sometimes variable naming isn’t a small detail. But sometimes it is and it’s a waste of everyone’s time to argue about it.

Writing readable code is not "nitpicking". It is common that an author doesn't see why their parameter name or function name is misleading but their reviewer, who comes to the change without as many preconceptions, sees it right away.

"Readable" and "misleading" are subjective, and depend on the person's "preconceptions".

I disagree that the reviewer comes "without as many preconceptions" - they just come with different preconceptions, the ones they're used to. Programming language is a language like any other, and each person has their own style of writing it, and they'd prefer the rest of the world to use their own style because it's "more readable".

Re: A study of Google's code review tooling (Critique)

#25

Post by compiler-guy »

raverbashing wrote: post #16

Wait... this looks like Gerrit with extra plugins The experience can be nice sure, but Gerrit is not something I'm too enthusiastic about (and yes, Github is better in some aspects, Gerrit is more configurable though and it seems it is smarting tracking "non official" merges)

Gerrit is a released version of Google's code review tool called Mondrian--the predecessor to Critique. Critique is basically Gerrit 2.0, although Gerrit has evolved somewhat independently.

Re: A study of Google's code review tooling (Critique)

#26

Post by fishnchips »

jeffbee wrote: post #9

Satisfaction with Critique among Xooglers is undoubtedly driven by dissatisfaction with GitHub PR reviews. GitHub reviews are astonishingly bad. The tool is utterly useless for actual reviews. After the first round of comments it becomes total chaos. Nobody can tell what's been said, done, changed, or resolved. It is impossible to believe that the people who write and maintain the GitHub PR tools have themselves ever…

To be fair to GitHub, there has been slow but constant progress over the last few years. I would love it to get at some point to where Critique was when I left Google (almost 10 years ago).

Re: A study of Google's code review tooling (Critique)

#27

Post by jeffbee »

marcinzm wrote: post #22
jeffbee wrote: post #19

Earlier quoted context omitted.

Google3 is a huge codebase of high quality that moves exceptionally quickly, so it just seems like your priors are getting in the way. Complaining that a large-scale change among tens of thousands of software developers requires a tool (Rosie) is sort of the same thing as all those people who complain that it's too hard to cope with having millions of machines in prod, i.e. the type of people who wash out of the comp…

The same applies to Facebook which is their comparison and not some random 10 person startup.

Except Facebook's codebase is an order of magnitude smaller.

Re: A study of Google's code review tooling (Critique)

#29

Post by ASinclair »

tallowen wrote: post #13

I’m always surprised to see the totality of support for this workflow. My biggest gripes were: - Owners - Readability review (or really the 18month queue to get Java/python readability) Although seemingly innocuous, this made maintaining internal libraries very challenging. There was no way to update every call site of your library in an efficient way. Tools like Rosie were added on top so that you could shard your P…

> I don’t think libraries like React could get developed at Google given how hard it would be to evolve the API surface.

That's more of a feature and not a bug to me. Users of a library would appreciate it if the API surface doesn't change dramatically over time.

Re: A study of Google's code review tooling (Critique)

#30

Post by tikhonj »

bheadmaster wrote: post #23
jeffbee wrote: post #10

Earlier quoted context omitted.

Writing readable code is not "nitpicking". It is common that an author doesn't see why their parameter name or function name is misleading but their reviewer, who comes to the change without as many preconceptions, sees it right away.

"Readable" and "misleading" are subjective, and depend on the person's "preconceptions". I disagree that the reviewer comes "without as many preconceptions" - they just come with different preconceptions, the ones they're used to. Programming language is a language like any other, and each person has their own style of writing it, and they'd prefer the rest of the world to use their own style because it's "more reada…

Are you saying there is no such thing as clearer or less clear writing in natural languages or programming languages? It's 100% subjective and depends entirely on the reader?
Post reply on HN