Live data from Hacker News

How to do a code review

google.github.io

11–20 of 376 posts

Re: How to do a code review

#11
post #5

Would help if they mention what a "CL" is somewhere before using it ubiquitously

It refers to "changelist". Google uses a Perforce-like VCS internally, and they kept the Perforce terminology: https://www.perforce.com/perforce/doc.051/manuals/p4guide/07...

aka "Pull Request" in Github parlance.

Re: How to do a code review

#13
This is great advice and isn't followed often enough, especially when reviewing code written by people new to an organization/team:

> If you see something nice in the CL, tell the developer, especially when they addressed one of your comments in a great way. Code reviews often just focus on mistakes, but they should offer encouragement and appreciation for good practices, as well. It’s sometimes even more valuable, in terms of mentoring, to tell a developer what they did right than to tell them what they did wrong.

I've seen cases where people got hundreds of comments (many of them minor, nitpicky) from more experienced developers and were discouraged by the sheer number of them. That most new developers naturally suffer from imposter syndrome is not helped at all by 100% critical code reviews.

Re: How to do a code review

#14
"Technical facts and data overrule opinions and personal preferences." So true, so true!

Now if teams pay careful attention to this statement, the Code Review Guidelines can be shortened by an order of a magnitude! ;-)

Re: How to do a code review

#15
post #6

As a side note, their github hosts 1600 repos. Just wanted to say thank you, google.

Wow, that is really a huge number of repos. I wonder how many are just forks though.

I will teach you how to check that: https://i.imgur.com/HjhAmQW.gif

Here are the statistics just in case anyone else is interested:

  1,519 Sources
     38 Forks
     18 Archived
      6 Mirrors
  --------------
  1,581 Total
Source: https://github.com/google

Re: How to do a code review

#16
post #6

As a side note, their github hosts 1600 repos. Just wanted to say thank you, google.

I would wager that a vast majority of them are personal projects. If you work at Google and want to do some open source project on the side, the code is owned by Google and is under Github.com/google. See: https://news.ycombinator.com/item?id=15592968

Re: How to do a code review

#17

"Technical facts and data overrule opinions and personal preferences." So true, so true! Now if teams pay careful attention to this statement, the Code Review Guidelines can be shortened by an order of a magnitude! ;-)

Code Review Guidelines are an amalgamation of said technical facts and data, data gained through experience of the company's entire lifespan, but also mixed in with a ton of personal preferences along the way. The trick is separating the two ;)

Re: How to do a code review

#18
In the "What Is Not An Emergency?" section:

> It is the end of the day on a Friday and it would just be great to get this CL in before the developer leaves for the weekend.

I laughed out loud because it reminded me of so many times I have seen it happen and then someone had to fix in the weekend.

Who shares the same experience?

Re: How to do a code review

#20

"Technical facts and data overrule opinions and personal preferences." So true, so true! Now if teams pay careful attention to this statement, the Code Review Guidelines can be shortened by an order of a magnitude! ;-)

As a general rule, absolutely! Unfortunately, if applied too strictly though, it can be weaponized or stifling in ways you may not anticipate (which tends to be true of almost any rule used too universally).

For example: I’ve seen cases where there’s disagreement as to which of two technical paths to pursue, where one side (not liking the way consensus is currently leaning) demands a data driven proof that one of the choices is best. Ordinarily this would be fine, but in this case, it would take no less work/time to gather said data than to entirely implement both solutions and directly compare them.

Post reply on HN