We are ruthless on code reviews • Hacker News Board

Re: We are ruthless on code reviews

#31

Post by computmaxer » Wed, Apr 27, 2016, 4:01 PM UTC

gjolund wrote: post #2

I love working in environments like this. I can say hands down they were my most productive years as a programmer. Quality code review is the single most important trait of a high performing senior programmer.

As someone who works at Workiva, I completely agree.

The post comes off a little more abrasive than I'd like. I think writing it as an "email" to a hypothetical new team member makes it worse.

The point is: we take our code reviews seriously and we may point out things that seem silly or nitpicky, we may question your approach, we may make many suggestions for improvement, but at the end of the day it is nothing personal. It is for the common goal of high code quality.

Re: We are ruthless on code reviews

#32

Post by brandmeyer » Wed, Apr 27, 2016, 4:05 PM UTC

auggierose wrote: post #28

It always astonished me how delicate coders in the UK are (I am from Germany, working in the UK currently). During a team meeting at a former company, my "ruthlessness" was brought up as a negativism. I countered that (or so I thought) with the following question to the rest of the coder team: "Do you want me to point out problems I perceive with the code / approach / whatever, or do you want me to hold back in order…

There is another side to this. I've seen a ton of technical criticism get delivered with emotionally laden commentary. X is "totally broken", "crap", "not even wrong", "just wrong", "terrible", etc. There is a way to deliver criticism while maintaining an environment of common professional respect. Many people don't even try. With the caveat that I'm painting with a culturally biased brush, I've found Germans (and Finns!) to be particularly abrasive in this regards.

Re: We are ruthless on code reviews

#33

Post by yolesaber » Wed, Apr 27, 2016, 4:06 PM UTC

MisterBastahrd wrote: post #6

If you've got time to dream up ways to be witty in order to be snarky to a co-worker, then you've got too much time on your hands at work and clearly need to fill it with other responsibilities.

That's right! Get back to work. That's all you're good for, after all. Just keep working! Have a funny thought you want to jot down? Sorry pal forget it you clearly should be working, endlessly, tirelessly. Don't forget you're here forever. Working.

Re: We are ruthless on code reviews

#34

Post by jbrooksuk » Wed, Apr 27, 2016, 4:06 PM UTC

As a PHP "shop" this is why we use StyleCI [1].

Our code isn't that critical, the quality and maintainability are. We don't waste time on "code reviews" that only check the style of code, that can be automated away.

Yes, I make StyleCI, but we use it in the place I work too (unrelated).

[1] https://styleci.io

Re: We are ruthless on code reviews

#35

Post by bobbytherobot » Wed, Apr 27, 2016, 4:07 PM UTC

gjolund wrote: post #2

I love working in environments like this. I can say hands down they were my most productive years as a programmer. Quality code review is the single most important trait of a high performing senior programmer.

Before people take and run with this comment, this will turn into a shit show if you don't have actual senior programmers with the needed experience. I've seen it happen in startups where their "senior programmer" has only been working for three years out of school for the single startup.

Re: We are ruthless on code reviews

#36

Post by dlandis » Wed, Apr 27, 2016, 4:09 PM UTC

Ugh, comes off as condescending, immature, and insufferable.

I do think strict code review and high standards are the way to go, but when implementing them it becomes all the more important to have appropriate and professional interpersonal communication and documented policies.

Re: We are ruthless on code reviews

#37

Post by skellington » Wed, Apr 27, 2016, 4:09 PM UTC

auggierose wrote: post #28

It always astonished me how delicate coders in the UK are (I am from Germany, working in the UK currently). During a team meeting at a former company, my "ruthlessness" was brought up as a negativism. I countered that (or so I thought) with the following question to the rest of the coder team: "Do you want me to point out problems I perceive with the code / approach / whatever, or do you want me to hold back in order…

It's interesting that even your comment is getting down voted. I wonder if it's a millennial thing? It would be interesting to know the ages of the people here commenting on how "harsh" the post seems vs. "yeah that's great."

I read the post as going out of it's way to give praise while pointing out what the company requires (perhaps that is interpreted as patronizing). But many people here are commenting on how rude it is. Bizarre. If you think this is rude, you've never worked in a "tough" environment.

How would people here suggest communicating the same information? Is it just that it might be interpreted as patronizing that's the problem?

Re: We are ruthless on code reviews

#38

Post by louden » Wed, Apr 27, 2016, 4:10 PM UTC

'We are rigorous on code reviews' would be a better approach. We don't have to be jerks to make sure that code is up to standards.

Re: We are ruthless on code reviews

#39

Post by ybobinator » Wed, Apr 27, 2016, 4:10 PM UTC

nmeofthestate wrote: post #22

Code reviews are pretty heavy going on the project I'm working on at the moment. The code isn't noticeably better for it though!

You gotta argue your side if what they're saying is going to make shit awful. IMO the best way to do it is to try their approach out and show them in a few lines that you don't think it's helping. Doing whatever people say just because is bad and so is just saying no. You gotta work with them.

Re: We are ruthless on code reviews

#40

Post by jkot » Wed, Apr 27, 2016, 4:10 PM UTC

For me declaration of "ruthlessness" is a red flag, similar to ninja buzzword.

Strictness in one direction does not mean company is strict at all fronts. Code reviews can be used without automated tests or version control.

In this case the review process seems like a "busy work". Checks bellow could be easily integrated into build script or CI. Such code should not even made it into code review. (I am not arguing about the rule itself, just how it should be enforced)

> You’ll notice that I left the comment “Beep!” on the imports of every file you touched. What I meant was, “Your imports violate our standard convention—we order them by built-ins, then third party, and then project level,” but that was too much to type on every file.