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

11–20 of 72 posts

Re: We are ruthless on code reviews

#11

Post by savanaly »

parennoob wrote: post #9

The whole "we are so cool because we are ruthless . Merciless , I tell you! Once you get through our code review your code will be perfect." attitude of this article is a bit surprising for one ranked so highly on HN. Do people really prefer this kind of attitude? Bender GIFs and snarky comments can become quickly tiring. I personally would strongly prefer if they put it in milder technical terms, "We are very conser…

Every single time I do a code review I find myself noticing a lot of little style mistakes (and, not to be overconfident, I could be mistaken myself about that) and don't leave a comment because I don't want to be that guy that no one wants to review their code and gives one a major headache because I care more about the consistency than does the person I'm reviewing.

In other words, I might restrain myself from criticism out of my compassion for my coworkers. The definition of "ruthless" according to Google is "having or showing no pity or compassion for others," so it's a fine term to describe how I'm not-- and how this company is determined to be.

Re: We are ruthless on code reviews

#12

Post by parennoob »

savanaly wrote: post #11
parennoob wrote: post #9

The whole "we are so cool because we are ruthless . Merciless , I tell you! Once you get through our code review your code will be perfect." attitude of this article is a bit surprising for one ranked so highly on HN. Do people really prefer this kind of attitude? Bender GIFs and snarky comments can become quickly tiring. I personally would strongly prefer if they put it in milder technical terms, "We are very conser…

Every single time I do a code review I find myself noticing a lot of little style mistakes (and, not to be overconfident, I could be mistaken myself about that) and don't leave a comment because I don't want to be that guy that no one wants to review their code and gives one a major headache because I care more about the consistency than does the person I'm reviewing. In other words, I might restrain myself from crit…

Yeah I think this kind of thing is a teamwide preference. If you prefer very rigid styling and enforce a style checker, great – that can be put into a script which you compulsorily run before review.

Re: We are ruthless on code reviews

#13

Post by estefan »

People find it acceptable to write more harshly than they'd speak face-to-face. I wonder if all 43 or whatever points would have come up if it was done side-by-side the developer.

Also, great negative PR for this company. This sort of discussion would also come off as much less arrogant/snarky face-to-face.

Re: We are ruthless on code reviews

#14

Post by eddieroger »

I guess that's one way of doing it, but you're dependent on the quality of said ruthless reviewers, and delivery of the message is separate from the message itself. Personally, I'm more of a "kill them with kindness" guy, so while my reviews would come off as soft by the article's definition, I'd argue that they're just as effective, and my teammates still want to have a beer with me at the end of the day, instead of walking away thinking I have a stick up my ass, which an email about ruthlessness wouldn't necessarily negate.

Re: We are ruthless on code reviews

#15

Post by siegecraft »

God, how insufferable. "Our code reviews are so ruthless because our clients (and by extension, us) are so much better than everyone else!" But apparently we can't write a bot or a pre-commit hook to re-order imports so instead we'll write some non-sensical in-house jargon (because typing out the full explanation is too much effort, even though we are the best of the best because our clients are so excellent and demanding). But don't worry, we'll write a 2 page email to explain this jargon.

Re: We are ruthless on code reviews

#16

Post by mkozlows »

savanaly wrote: post #11
parennoob wrote: post #9

The whole "we are so cool because we are ruthless . Merciless , I tell you! Once you get through our code review your code will be perfect." attitude of this article is a bit surprising for one ranked so highly on HN. Do people really prefer this kind of attitude? Bender GIFs and snarky comments can become quickly tiring. I personally would strongly prefer if they put it in milder technical terms, "We are very conser…

Every single time I do a code review I find myself noticing a lot of little style mistakes (and, not to be overconfident, I could be mistaken myself about that) and don't leave a comment because I don't want to be that guy that no one wants to review their code and gives one a major headache because I care more about the consistency than does the person I'm reviewing. In other words, I might restrain myself from crit…

As a team, decide how much you care. Some teams won't care much at all, and will follow a "hey as long as it works" attitude. Other teams will be deeply interested in making their code as good as it could be, and will want comments about doing things more cleanly or elegantly and will view PR review as a chance to learn and improve.

As long as everyone's on the same page, and sharing the same attitude, that drive to improve will come across as a positive thing rather than a jerk move. But if you're the one person with that attitude on a team of good-enoughers, yeah, not so much.

Re: We are ruthless on code reviews

#17

Post by hvidgaard »

joesmo wrote: post #7

Yet despite being assholes (ruthless in their parlance) who obviously have not on-boarded said employee or even told him what is expected of his code, I doubt their software has any less bugs than the software from shops who are not assholes (ruthless in their parlance). Sounds like a great reason to avoid working here and other places that have the same mentality. If you want your coworkers to actually do their work…

In this articles terms, I'm ruthless when doing code reviews. I do it because

* We have a code style to follow, and if we don't we might as well not have it.

* It ups the game from the developers, they double check their code before checkin, and believe it or not, they catch more errors this way.

* It have created an environment where we can openly talk about interative improvements.

Re: We are ruthless on code reviews

#18

Post by scolfax »

- If this post is satire, it's hilarious and spot-on!

- If this is real, quit while you can, and may god have mercy on your soul.

- The fact that it's difficult to tell if it's real or not says a lot about our industry.

Re: We are ruthless on code reviews

#20

Post by coredog64 »

Niksko wrote: post #8
noamsml wrote: post #5

Earlier quoted context omitted.

Playing devil's advocate here: The only way to really learn the full ruleset of a team is by undergoing a couple of code reviews in which every issue is pointed out. Once your teammates see you've understood the prevailing style, they don't have to work as hard at code reviews. That said, I usually hate petty/nitpicky code reviews. I try to focus only on issues that I think affect readability, maintainability or func…

Makes sense, but there are ways of being even petty and nitpicky without being condescending or rude. State facts, don't throw insults, don't demean or belittle.

One good rule I was given by a tech lead is to not use the word "you" in the PR comments. PR comments are a discussion on the failure of the code, not of the individual. Avoiding "you" allows for some separation between the two.
Post reply on HN