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

We are ruthless on code reviews

techblog.workiva.com

Post reply on HN
Print view View original

61–70 of 72 posts

Re: We are ruthless on code reviews

#61

Post by [deleted] »

burkaman wrote: post #48
skellington wrote: post #37

Earlier quoted context omitted.

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…

Yes, it's extremely patronizing. A better email might be: "Hey, sorry if that code review seemed a little nitpicky. You did a great job, but we try to be as strict and consistent as possible in our reviews, for everyone's benefit. Here's a link to our style guide, and please let me know if you have any questions. I'll stop by your desk in a little bit to introduce myself and see how you're doing." You don't need any…

[deleted]

Re: We are ruthless on code reviews

#62

Post by estefan »

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 not what you say it's the way that you say it.

If you don't have the social acumen to know how to raise issues and to discuss them reasonably (i.e. without rubbing people up the wrong way), then yes, erring on the side of being considerate is likely to win you more friends and help you keep your job longer.

There's a fine line between constructive feedback and being toxic to a team.

Re: We are ruthless on code reviews

#63

Post by raverbashing »

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…

I'm glad you wrote this because I would definitely never consider a person with such an obtuse attitude for any position

It's not about sensitivity it's about not being a inconsiderate pedant

I'm also sure that a lot of your critique has no basis in technical quality rather than personal preferences. This is what I see time and time again with the people that "worry most" about the code (but when it's their code that's critiqued it's full of problems). Uncle Bob fans, "code craftsmen" all those people make my BS detector beep

There are also people that pick a lot of "problems" in the code that ignore glaring issues in other areas/code they wrote/other aspects of the issue, including further code maintainability, readability (no, being readable to you does not mean it's readable by somebody else), etc

It's not so much about bruised egos rather than wasting company time with pedantic nitpicking, not trusting colleagues (which is fundamental and you seem to be utterly incapable of), overstating your own importance in the team (unless you really picked a company full of bozos, which in the end is your mistake again)

Re: We are ruthless on code reviews

#64

Post by borvo »

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…

So they didn't ask you to point out the problems in a more considerate way?

Instead, they just asked you to stop pointing out the problems?

Were these problems nitpicks, personal preference (as some here are apparently assuming) or were they objectively technical issues?

Re: We are ruthless on code reviews

#65

Post by yongjik »

auggierose wrote: post #55
gdulli wrote: post #46

Earlier quoted context omitted.

Programming tends to attract a lot of people who are smart but too young to have wisdom. The kind of wisdom that tells one when it's worthwhile to be ruthless and when it's just pedantry, self-defeating, etc. The kind of wisdom that keeps one from positing that the people who don't see things their way are delicate.

Wisdom is overrated. Also, I am probably older than you are :-) I am using "I" and "You" because I like direct communication instead of passive/aggressive snobishness.

> ... because I like direct communication instead of passive/aggressive snobishness.

See, wisdom is not saying stuff like this.

Re: We are ruthless on code reviews

#66

Post by Ensorceled »

crdoconnor wrote: post #57
Ensorceled wrote: post #50

Earlier quoted context omitted.

Until the first time you spend a couple of hours looking for a weird bug in Python because somebody broke this rule...

Which rule? The rule that imports must be ordered according to the whims of the team lead? Or the rule that lines must be under 80 characters? There are a few linting rules which it pays major dividends to pay attention to (e.g. variables that are initialized but never used), but most of them are superficial.

You commented on the import ordering so clearly that's what I'm commenting on.

Not following the PEP standard for python import ordering can cause issues like circular imports or monkey patching not working properly.

Re: We are ruthless on code reviews

#67

Post by auggierose »

raverbashing wrote: post #63
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…

I'm glad you wrote this because I would definitely never consider a person with such an obtuse attitude for any position It's not about sensitivity it's about not being a inconsiderate pedant I'm also sure that a lot of your critique has no basis in technical quality rather than personal preferences. This is what I see time and time again with the people that "worry most" about the code (but when it's their code that…

I am all against pedantic nitpicking. One of my most common complaints was to concentrate on the actual problem, not on the style. I am not a "code craftsman", I am somebody who knows a lot about how to solve a problem with code. And I don't work for anyone except myself anymore.

Re: We are ruthless on code reviews

#68

Post by gjolund »

bobbytherobot wrote: post #35
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.

While I understand where you are coming from, I don't see how your comment adds anything to the conversation.

If you don't have senior staff then you are going to have a host of other problems as well.

Re: We are ruthless on code reviews

#69

Post by gjolund »

computmaxer wrote: post #31
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 pers…

Code review is typically how I structure my interviews.

I try not to ask arbitrary comp-sci questions, but instead provide the candidate with a block of code and ask them to provide a comprehensive code review (usually through a github pr).

Afterwards I perform a my review, and highlight any differences in style or methodologies. I'm not really looking for people that have the exact same outlook as me, I'm just looking for people that have some kind of standard they adhere to.

Re: We are ruthless on code reviews

#70

Post by whisky_jb »

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 is very difficult to do code reviews when beside the technical topics politics come into place. After 8h of reviews on 2000 lines of code finding 100 issues, it is almost not possible to be nice anymore. But hell starts when manager decide to give a GO because deadline approached without even fixing the 3 major bugs.
Post reply on HN