Ask HN: How do you build a healthy code review culture? • Hacker News Board

Re: Ask HN: How do you build a healthy code review culture?

#11

Post by thisone » Thu, Mar 10, 2016, 7:19 PM UTC

if you need to get heavy handed about it, people are probably going to leave. Which may be a good thing, since you shouldn't have to get heavy handed about simple things like styling.

I'm a big fan of strong linting where failure to pass linting will fail the build.

The general reason why I feel that way isn't anything to do with readability but because linting can teach you what NOT to do and make you a better programmer.

Code reviews shouldn't be "move this comma" let linting handle that.

Code reviews can then be more "is what this is doing sensible, are the test cases covering the change, and (possibly most importantly) do I understand how this changes the application"

Re: Ask HN: How do you build a healthy code review culture?

#12

Post by gumby » Thu, Mar 10, 2016, 7:32 PM UTC

grif-fin wrote: post #6

over wrote: post #3

Earlier quoted context omitted.

I really like this, it's process engineering applied to software engineering. But how do you do a process review? :)

How easily did everyone agree to the coding standard purposed? I would be surprised and would have liked to see that coding standard if everyone agreed in the company with it. I remember starting a project with close friends who knew each other well but we struggled on agreeing on some coding standard which everyone was happy. People have different taste when it comes to formatting the code syntax and unless they rea…

> How easily did everyone agree to the coding standard purposed?

We had a meeting when the programming team was relatively small. One of us proposed we use the Google standards -- not because he liked them (and he was an ex googler) but simply because they were there and were reasonably comprehensive. He sent the link around before the meeting.

In the meeting we went around and all of us (including the proposer) described something in the google standards that we didn't really like, but in the end could probably live with. For all intents and purposes this was venting, though it wasn't as heated as "venting" usually is.

Then we agreed on the following: - If we used an existing piece of code, we followed its coding standards - For our code we used the Google C++ standard - We had some C# (mono) code so we made a couple of rules for it based on the google standards. - Over time we added a couple of rules specific to us.

> we struggled on agreeing on some coding standard which everyone was happy.

We didn't worry about that. We were happy to have something rather than it be somehow the be all and end all. We chose Google's because it wasn't terrible and of all the standards it was the most likely to already be known by a new hire.

It's the same way we settled on git -- I like it, some hate it, but it does meet our code needs and most people have it on their resume already. Were it doesn't work (e.g. electrical and mechanical CAD, or big binary assets like UI) we don't use it.

Re: Ask HN: How do you build a healthy code review culture?

#13

Post by seanwilson » Fri, Mar 11, 2016, 4:11 AM UTC

EliRivers wrote: post #2

People get precious and personal about their code. They do. Some of them take any critique very personally. Some of them have a tantrum, some of them just ignore anything anyone says. Programming ego is a real problem. A couple of jobs ago, I worked somewhere with an excellent code review culture, and it began with a simple, written standard, very small, that everyone had agreed to. I now refuse to review code unless…

"If there's no document to point at, no objective standard to meet or not meet, I'm not reviewing it because it's just not worth the tantrums and screaming from precious programmers."

What about rules for less objective things like how readable a function name is though? If someone wants to be difficult addressing code reviews, there's always plenty of subjective feedback they can do this for.

Re: Ask HN: How do you build a healthy code review culture?

#14

Post by panic » Fri, Mar 11, 2016, 6:59 AM UTC

Does management understand the benefits of a healthy code review culture? Managers often support code review in the abstract, but when faced with an actual decision between respecting code review or shipping a feature faster, they'll choose to ship faster. Your colleagues are probably just responding to this.

Re: Ask HN: How do you build a healthy code review culture?

#15

Post by kat » Fri, Mar 11, 2016, 10:49 PM UTC

I worked at a company that started having code reviews. We started off by just "presenting" code to the team. The team would point out any thing that looked like a bug or areas that required extra QA effort. Just knowing you would have to present your code was enough to increase code quality. We didn't worry about style/best practices/subjective feedback at the beginning. Over a few months we all became more comfortable receiving and giving feedback and that's when we started to solidify the style guidelines. Also our ability to catch bugs without QA cycles helped increase buy-in for code reviews.

Re: Ask HN: How do you build a healthy code review culture?

#16

Post by dmm » Sat, Mar 12, 2016, 12:13 AM UTC

seanwilson wrote: post #13

EliRivers wrote: post #2

People get precious and personal about their code. They do. Some of them take any critique very personally. Some of them have a tantrum, some of them just ignore anything anyone says. Programming ego is a real problem. A couple of jobs ago, I worked somewhere with an excellent code review culture, and it began with a simple, written standard, very small, that everyone had agreed to. I now refuse to review code unless…

"If there's no document to point at, no objective standard to meet or not meet, I'm not reviewing it because it's just not worth the tantrums and screaming from precious programmers." What about rules for less objective things like how readable a function name is though? If someone wants to be difficult addressing code reviews, there's always plenty of subjective feedback they can do this for.

The Google c++ styleguide[0] has some examples, in general and specifically for files, functions, etc. Looking over them most are pretty objective so maybe it's worthwhile to come up with some usable rules that enforce qualities you notice readable code shares, even if they aren't completely comprehensive.

[0] https://google.github.io/styleguide/cppguide.html#Naming

Re: Ask HN: How do you build a healthy code review culture?

#17

Post by eecks » Sun, Mar 13, 2016, 1:49 PM UTC

I kinda like reading stuff like this online. It reminds me that I (and my colleagues) are doing this stuff extremely well.

The only way to change your culture is to draw up standards and then fail any code review that does not meet the standards. Nothing goes live until it passes code review.