Hacker News new | ask | show | jobs
by EliRivers 3756 days ago
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 I am able to say "This code doesn't conform to the document you agreed to; it's not that I think this is bad code, it's that it doesn't meet the document that you agreed to." 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.

There also need to be consequences for code that doesn't meet the document; basically, it's not ready to merge/commit until it does. There needs to be a way to allow code that violates the standard to be waived and permitted, with agreement from the developer and reviewer, with the waiver recorded so the developer feels listened to and the reviewer feels protected.

You're going to have to get buy-in for this from someone with power and authority.

2 comments

"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

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 really deep down agree to coding standard they may still like their previous habit they keep going back to them.

Overall I think it is tricky and I sometimes found it the argument of I like blue and you like red, what should we do now? Want to convince me to like red?

How easily did everyone agree to the coding standard purposed?

We did it by making it really, really small. No style guidelines. Two space, four spaces, braces in line, braces not in line? Don't care. It doesn't matter. If you're working on some code and you genuinely can't understand it because the layout is a mess, dump it through an auto-layout tool or the like. Honestly, so long as it's readable, arguing over two spaces for an indent or whether the if block first brace should be on the next line or not isn't worth the hassle.

We started with a handful of outright forbidden functions, and some rules on memory management. As time goes on, and various mistakes get made repeatedly, protections against them get added to the standard.

A small standard is better than no standard. A small standard is also better than a big standard.

> 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.