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

Software Engineering at Google

books.google.com

Post reply on HN
Print view View original

51–60 of 73 posts

Re: Software Engineering at Google

#51

Post by jbn »

joshuamorton wrote: post #43
kortilla wrote: post #41

Earlier quoted context omitted.

The comment I replied to is literally highlighting people nit-picking punctuation as if it’s a good thing.

Which it is! You seem to be assuming, without prompt, that nit-picking punctuation comes at the expense of a thorough overall review. Code review on punctuation is absolutely an important part of readability. Consistency makes understanding and scanning (for human readability) and potentially parsing and modifying (for machine readability) much easier. As someone who both reviews a lot of code and reads a lot of code…

what about nitpicks related to whitespace? These are an endless source of time-wasting, IMHO.

Re: Software Engineering at Google

#52

Post by joshuamorton »

jbn wrote: post #51
joshuamorton wrote: post #43

Earlier quoted context omitted.

Which it is! You seem to be assuming, without prompt, that nit-picking punctuation comes at the expense of a thorough overall review. Code review on punctuation is absolutely an important part of readability. Consistency makes understanding and scanning (for human readability) and potentially parsing and modifying (for machine readability) much easier. As someone who both reviews a lot of code and reads a lot of code…

what about nitpicks related to whitespace? These are an endless source of time-wasting, IMHO.

At Google or in general?

The closest I've seen to this is comments of the form "can you reformat this" or "this doesn't match the rest of the file". Google's style guides have fairly strong guidance on formatting, so there's usually a "right" way (enforced by a linter), and if not, then usually you defer to the existing style in the module.

Re: Software Engineering at Google

#53

Post by ablekh »

tituswinters wrote: post #49
ablekh wrote: post #34

Not providing table of contents is IMO offending to potential readers. And, unfortunately, this issue applies not only to this book.

https://learning.oreilly.com/library/view/software-engineeri...

Thank you for the link, though I strongly prefer to know full content structure information, i.e, all relevant sections' page numbers. This allows me to assess how much relative attention is given to a particular topic. I believe that a full table of contents should be a mandatory element in visual book previews for all book publishers and sellers (for outlets lacking visual previews, this information could be provided in text format).

Re: Software Engineering at Google

#54

Post by akavel »

nostromo wrote: post #13
akavel wrote: post #5

Earlier quoted context omitted.

I think in such cases it's because people probably use upvotes as bookmarks to "read later" ( ...some day, for sure! )

Just so people know, you can use "favorite" for that. You can also favorite comments.

AFAIK, favorites count as upvotes as well.

Re: Software Engineering at Google

#55

Post by jbn »

joshuamorton wrote: post #52
jbn wrote: post #51

Earlier quoted context omitted.

what about nitpicks related to whitespace? These are an endless source of time-wasting, IMHO.

At Google or in general? The closest I've seen to this is comments of the form "can you reformat this" or "this doesn't match the rest of the file". Google's style guides have fairly strong guidance on formatting, so there's usually a "right" way (enforced by a linter), and if not, then usually you defer to the existing style in the module.

I meant in general. Except python-like languages that make it a non-issue, in C/C++/Java I think people attach way too much importance to spacing, trailing whitespace, tab vs spaces, and column width.

I would agree with linting or just deferring to existing style, but in truth I care little about this, it's just not significant to me (yet others seem to overvalue it, to the point of talking exclusively about this).

Re: Software Engineering at Google

#56

Post by Shish2k »

ridiculous_fish wrote: post #37

Xoogler here, circa 2015. My reaction is that this is a very google3 (i.e. web services) centered book. But Google contains multitudes and it feels wrong to ignore them. For example the book has a section called "How Code Review Works At Google." And it goes on to describe strictly the google3 process. But Chrome, ChromeOS, GoogleX, others have different processes. If Google has a proven model, why do so many of its…

> we don't require Readability.

I assume from capitalisation and context that Readability is some very specific and very strict set of rules? I’m curious what would be there that would be so offputting...

Re: Software Engineering at Google

#57

Post by thedance »

Shish2k wrote: post #56
ridiculous_fish wrote: post #37

Xoogler here, circa 2015. My reaction is that this is a very google3 (i.e. web services) centered book. But Google contains multitudes and it feels wrong to ignore them. For example the book has a section called "How Code Review Works At Google." And it goes on to describe strictly the google3 process. But Chrome, ChromeOS, GoogleX, others have different processes. If Google has a proven model, why do so many of its…

> we don't require Readability. I assume from capitalisation and context that Readability is some very specific and very strict set of rules? I’m curious what would be there that would be so offputting...

The readability process at Google is a way to gain the ability to submit code without a language style reviewer. To submit code into google3 (their main repo for server-side stuff), a changelist requires acceptance by a person or people who collectively own all of the code being changed, and a person or people who collectively "have readability" in all of the languages in the change. The simplest example is there is some C++ code that you own and you have C++ readability. In this case you need the review of anyone. If you don't have C++ readability, you need a reviewer who does have it, and they need to carefully review your change for style.

It used to be that you had to do a "readability review" which was a large change that you developed specifically to gain readability in some language. A style reviewer would approve it after (usually) many rounds of comments, and then thereafter you had C++ readability forever.

More recently some language teams have adopted incremental readability. In this process you add a language reviewer to all of your changes, and after you demonstrate that you know what you are doing, they will grant you readability. I think the Go team was the first group to adopt incremental readability, but it spread to other languages.

This process is off-putting to people who want to join Google but not be acculturated to their language style rules. There's a surprising number of these people. In mainline Google these people simply will not get bonuses, raises, and promotions. In defective organizations like Android they just discard Google code quality standards and it shows.

Re: Software Engineering at Google

#58

Post by thedance »

jbn wrote: post #55
joshuamorton wrote: post #52

Earlier quoted context omitted.

At Google or in general? The closest I've seen to this is comments of the form "can you reformat this" or "this doesn't match the rest of the file". Google's style guides have fairly strong guidance on formatting, so there's usually a "right" way (enforced by a linter), and if not, then usually you defer to the existing style in the module.

I meant in general. Except python-like languages that make it a non-issue, in C/C++/Java I think people attach way too much importance to spacing, trailing whitespace, tab vs spaces, and column width. I would agree with linting or just deferring to existing style, but in truth I care little about this, it's just not significant to me (yet others seem to overvalue it, to the point of talking exclusively about this).

At Google (in google3) there are automatic linters that run on every change and if they fail then your change simply will not be approved for style. There's no debate about it. This is one of the great things about gofmt: either your Go program passed through it, or it did not and won;'t be accepted. Arguments about the formatting need to be in the form of a changelist against gofmt itself.

Re: Software Engineering at Google

#59

Post by shaftway »

jbn wrote: post #55
joshuamorton wrote: post #52

Earlier quoted context omitted.

At Google or in general? The closest I've seen to this is comments of the form "can you reformat this" or "this doesn't match the rest of the file". Google's style guides have fairly strong guidance on formatting, so there's usually a "right" way (enforced by a linter), and if not, then usually you defer to the existing style in the module.

I meant in general. Except python-like languages that make it a non-issue, in C/C++/Java I think people attach way too much importance to spacing, trailing whitespace, tab vs spaces, and column width. I would agree with linting or just deferring to existing style, but in truth I care little about this, it's just not significant to me (yet others seem to overvalue it, to the point of talking exclusively about this).

Every team I've been on at Google has presubmits that enforce this, along with command line tools to "fix" it. There's no arguments about spacing. Nobody really cares about extra lines, but if you use tabs instead of spaces you're just wrong and your CL can't be submitted.

It's actually nice. I disagree with some of the rules and would like them to change. I think that if your if-block has a single statement and there's no else, and it all fits in a line then it's fine to inline it.

    if (theAnswerIsKnown) return theAnswer;
But I'm wrong. Because the linter doesn't allow it. So I moved on and I don't bother fighting that fight.

Edit: To be clear, I might be right, and this might be better, but I'm "wrong" in the sense that it's not linter compliant.

Re: Software Engineering at Google

#60

Post by shaftway »

thedance wrote: post #57
Shish2k wrote: post #56

Earlier quoted context omitted.

> we don't require Readability. I assume from capitalisation and context that Readability is some very specific and very strict set of rules? I’m curious what would be there that would be so offputting...

The readability process at Google is a way to gain the ability to submit code without a language style reviewer. To submit code into google3 (their main repo for server-side stuff), a changelist requires acceptance by a person or people who collectively own all of the code being changed, and a person or people who collectively "have readability" in all of the languages in the change. The simplest example is there is…

The way I've described it to people is that you need to get at least one review from someone who...

- is an Owner of the code

- has Readability in the language of the code

- is not you

If you are an owner and have readability, you just need someone who isn't you to sign off on it. If you don't have ownership or readability you need to get someone who has those things to sign off. If you can find someone who has both, great. Otherwise you'll need two people.

Having readability in a language can make you pretty desirable, particularly if it's in a language that your team doesn't often use. It's hard to get, and it takes time. My team is pure Java, but once in a long while we check in Python scripts. Most people have Java readability, but I'm one of two people on the team with Python, which makes me more valuable to the team.

It's really hard to get someone who isn't on your team to give you a readability review.

Post reply on HN