This guy ties his ego to his code, and assumes everyone does the same. The proposed solution is to not devalue the person by devaluing the code.

This seems exactly backwards to me.

Disconnecting ego from the work was the first big lesson I had when I started working in software. I hear parallel ideas from friends across industries, in fact an electrical contractor explained to me how he expects it of his apprentices just a few days ago.

I once had a joint software team with a client and put my foot in my mouth assuming they separated code and ego the way we did. The first time I reviewed some of their work, there was a design decision that I sorta assumed was the best of a few bad options. So I asked the developer why they did it that way.

I just wanted to hear some reason, any at all would have done. Something special about that approach, or that they considered the others and found it was, in fact, the least worst. Then we'd just move on, happy enough with our implementation. I'm used to doing that (and having it done to me) dozens of times a month. But what I got was not a defense of the design decision, and more a defense that he made a design decision. I touched a nerve. I now know to hedge, apologize before giving feedback and ask questions by asking around them. I don't really think it's an overall plus. Especially when the business domain puts high standards on us.

I wish I had 100 up votes for this quote: "Disconnecting ego from the work was the first big lesson I had when I started working in software."

Until you can do that, you'll always be a lesser software developer. You'll concentrate on maintaining your ego instead of developing the best software for the problem at hand. In a way that's the gist of the article.

It's interesting, but at the company I've worked at, where we've had dozens of senior developers come and go, we've never had this problem. Our approach to code reviews is purely educational. "Here's how we do it so we have the least amount of trouble understanding each others code." Everyone seems to be good with that, and egos get left at the front stoop. I've learned lots, I've taught lots, and I've never felt better than anyone or worse than anyone. It's all about the learning.

And let me tell you, when you hire some junior dev, and he gets these code reviews, and 3 years later he's a senior dev and anchoring a project, it's a pretty good feeling. I think better than any feeling you can get from lording knowledge over others.

> Until you can do that, you'll always be a lesser software developer.

Reminds me of a joke-ish comment I saw on here some time ago: The difference between a junior and senior developer is the willingness to say "that bug was totally my fault".

Also, "maybe we shouldn't build that".

Also, "No, it's not worth all of the hacks and shortcuts it would take to get it done in the given time. We need to cut features, not quality."

lol

Well I guess that makes me a senior developer at heart despite never holding a development job

> Disconnecting ego from the work was the first big lesson I had when I started working in software.

100% agree. Any code you write for your employer is not your code - you should be comfortable leaving it forever tomorrow if a better opportunity presents itself elsewhere.

It's a theme I hear a lot when I see interviews of powerful business people - you don't do anyone a favor by pretending something that is bad is good.

Agreed that the OP took the wrong lesson - the fix is not to lower standards, the fix is to learn to deliver honest and complete feedback with empathy.

I've had my fair share of controversial code reviews, both on the giving and receiving end.

I no longer ask 'why?' inside a code review, because it immediately puts the author in the position of justifying what they did, while feeling like they have to defend a decision.

Instead, I'll take a coaching (or a more socratic) approach that doesn't risk making the author feel dumb, or like they never considered the alternatives: "what was your reason for this?", "did you try x,y,z?", "I wondered if this would work?", "what is this for?", "what does this do?"

In addition to that, if I have a suggestion (or a suqqestion [0]), I will always follow it up with a refactored, copy/pastable code snippet to make it clear what I'm talking about and also to help push the review forward. This is especially useful for learning by example.

This means that code review takes more time and effort, but that investment of time has value. Code reviews become an implicit, shared mentoring space, and people like it when they see your review pop up.

[0] https://www.urbandictionary.com/define.php?term=Suquestion

I find it fascinating that people don't immediately translate "what was your reason for this?", "did you try x,y,z?" and "I wondered if this would work?" to "why?" "why?" "why"?

Yeah I had to learn this lesson, and I think I will continue to find my ego cropping up in strange places, and again practice letting go.

My ego drove me to work extra hard to try and impress others. I would become petulant if I didn't get my way. I look back on the first few years of my career with so much cringe!

Couldn't agree more -- one of my first real important lessons in the industry on the soft skills side of things. What went along with that was that it really took a while to sink in and figure out not just how to personally avoid it, but in a team setting how to help build a culture of that.

> Disconnecting ego from the work was the first big lesson I had when I started working in software

I wish it was my first big lesson, it took me way to long to learn and my mental health suffered greatly until I did.

“Disconnecting ego from the work was the first big lesson I had when I started working in software” This is so important in Design as well.

"If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel ... And if you tell me that you haven’t had this feeling ever, then you’re lying."

I can tell you I haven't had this feeling ever, but not for reasons I'm proud of. I hate doing code reviews. I have hated doing every one. I have disliked having to type every comment I have made on a code review. Each time I hope that everything is good enough and I can just approve it. And I won't comment unless I am convinced it is important.

I don't hate the coder, or the process that requires code reviews. I know code reviews are very important for many different reasons, so I don't shirk them, but I have become afraid of being an expert on a large team as I will have to do more of them. Not sure what is wrong, I love coding. Reviewing good code isn't so bad, reviewing bad code takes all the joy out of coding for me. If it's bad enough, I know that I need to make a lot of comments, and I'll need to make some more when it comes back with changes. I think we are just wired differently.

EDIT After further thought, I think a fundamental difference is I really want to see my coworkers succeed, and don't enjoy their failure. Also I dislike all forms of toil. But I think maybe part of the authors problem is not able to comprehend that some people can be nice like that, genuinely. In fact, I assume my coworkers want me to succeed as well, and I really think most of the time they do. But I am glad he decided to start acting that way, which must be especially difficult if you don't believe others will treat you the same way.

I can't say I enjoyed code reviewing but I never felt like I was being mean to my teammates. I was really happy when a teammate found a bug in code review of my code rather than that bug getting checked in. I was also really happy when they shared knowledge and I learned something. Maybe they shared a pattern I was unfamiliar with or maybe they pointed out an existing function I could use I didn't know was in the code.

I assumed the same was true for them. I learned from another team member to try to phrase comments as questions. "Did you mean to do X here? It seems like it might have issue Y" etc...

Bugs are easy.... you just point out the bug and move on... design flaws are hard... it's hard to bring up the fact that while yes, your module solves this one immediate problem, it was written so rigidly that two sprints from now the whole thing will need to be re-written scratch. Or maybe you took a particular approach that seemed easy to implement, but it won't do async stuff properly and so it needs to be re-worked from the ground up. And I get you solved your immediate problem for your immediate task, but whoever works on this next is going to have to completely re-think the approach and that's not theoretical, that next week when we add more behavior.

You ever worked on a team a where someone would go write code in their own crazy way that wouldn't follow any sort of existing pattern or take advantage of existing tooling? So they spend like a week on a simple task because they re-wrote the strings.c because they didn't want to include it? Yeah, they get fired eventually, but they are the worst to write code reviews for, because you go into it just thinking WTH, why are they doing it this way, this entire approach is convoluted and error-prone and rigid and fragile and complicated.

If a super engineer has advice for me on how to deal with this stuff, I'm all ears. I usually just ignore it unless it directly impacts me and then go back and re-write it when we have to add on to it/make it interop with some more code/release it as an available API.

I had a coworker who had a 2,000 line PR that took me 3 days to review, it was like code breaking. Custom abstractions upon custom abstractions, abstractions in custom imported utilities for a one-liner that was only used once. Basically, instead of writing the code, he made code that generated the code that was needed, once the program was running. Dear Lord. Like somehow this monstrosity was good because instead of creating the feature, he built tooling to make many identical features whenever we want! Except nothing can be different, if it's different we need to rewrite the whole thing.

Every 4 hours or so I would ask my boss if I really had to do it. He told me to just keep going, I think he was building a case for letting him go. I still only made like 10 comments, needless to say they weren't taken well. What a nightmare.

Tangential to this whole thread, but guys like that, every day he worked took two days for other devs to fix/undo his work. Took my company a few years to catch on, he was a senior dev and also good at talking himself up. I would have paid him the same salary to go sit on a beach somewhere and enjoy himself instead of touching the code, we'd all have been happier. In fact the world would be a better place, because now he is presumably working somewhere else doing the same things.

I hated code reviews when I first had to do them because it was a toxic place. I hate them less upon focusing on what you describe. I'll add to it that I've taken a more conversational tone. Sometimes I'll just add comments to talk about how this code fits into the whole system and weird legacy stuff that is affecting what they are writing.

> After further thought, I think a fundamental difference is I really want to see my coworkers succeed, and don't enjoy their failure. Also I dislike all forms of toil.

I'm in both of these camps too. I think I'd dislike code reviews if I didn't remind myself:

* Code my coworkers write is code I don't have to write, so it's saving me toil.

* Issues I catch in review are issues that won't lead to late night debugging sessions catching heisenbugs on the eve of shipping our product, so it's saving me some of the worst kinds of toil.

A little work now to save me a lot of work later. And I've been burned by suddenly getting stuck maintaining & bugfixing code that I got lax about reviewing. Each issue I catch, I think "thank goodness I caught that now - that'd be a pain in the ass to debug".

It also makes me genuinely appreciate review feedback catching all the dumb shit I might do, which can really help when your quick change turns into a slog of missed edge cases, "can you fix X while you're in there", etc.

Turn it around - make sure each person has the chance to review one anothers' code, and make sure the senior members of the team lead the way in using those reviews as opportunities to both highlight great things in the code and to propose changes /in a way that teaches/ how you're analyzing it and what made you propose that change. If they challenge it, great! They're thinking about your reasoning on their own, and you can use that to come to a new consensus on the team's approach to development.

There's great health to be gained from code reviews. But they really do need to practiced, reconfigured, and practiced again so your team gets what they need from it.

While I have similar feelings, I enjoy code reviews on high quality code. I like reading high quality code. But yes, the enjoyment of a code review is inversely proportional to the amount of feedback required. Some people enjoy mentorship more than others, and that's ok.

> "If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel"

This quote made me laugh from the exaggerated "insane pleasure" in other's mistakes. But I feel this way sometimes. I'm completely sure it comes from my own insecurities, so that when I see another guy missed something I wouldn't, I get a feeling of relief - actually I'm not the worst programmer in the world.

So for me this feeling comes from impostor syndrome - I thought I was bad, I saw another guy who was worse, I felt good. Insecurity is really a self-fulfilling prophecy. I'm trying to work on it by forcing myself to ask questions I think are dumb, and not fearing judgment from others. This in turn will also influence my peers to feel less afraid of their doubts, turning the spiral around.

A study from Google [1] agrees with this: they found that the best predictor of team quality was the willingness to ask questions and express opinions without fear. Quoting Google:

"In a team with high psychological safety, teammates feel safe to take risks around their team members. They feel confident that no one on the team will embarrass or punish anyone else for admitting a mistake, asking a question, or offering a new idea."

[1] https://www.inc.com/justin-bariso/google-spent-years-studyin...

I find using automated tools as part of the pipeline, like codefactor, are a decent first step to sort out some of the glaring issues. As they're automated and act as quality gates before an actual human comes to do a review then they're not as bad as they could be. Sure there scope can be limited, depending on what language/framework you're using, but common ones catch quite a lot of obvious issues.

I do tons of code review and enjoy it. I approach it with both learner/teacher attitude (not critic) and treat it as project learning and communication exercise.

Best way of doing code review is by asking questions.

Recently coworker recommended this talk which suggest good attitude IMO - you can find it under "strong code review" from rails conf

> And I won't comment unless I am convinced it is important.

I tend to lean this way as well. Im of the mindset that most things that are "caught" in a code review don't add any real value. Or maybe I just have bad coding standards.

This is incredibly honest. Maybe it's just me, but it 100% resonates with me. I find myself doing the exact same thing: as soon as I feel myself mounting up on my high-horse I have to talk myself down from being an asshat. I've gotten better at it because I'm now responsible for people's careers, and my pettiness is no reason to make someone's life suck because they are still learning -OR- because they think differently than me and have a different solution that I might not even UNDERSTAND. Being open-minded is really fucking hard for me some times.

These comments stuck out:

"Because I do code review for self-identification."

and

"It turned out that, instead of becoming a good coder, you simply have to convince others you’re a good coder. This behaviour begets a vicious cycle that produces not professionals, but toxic asshats."

One way to nip this culture is stop hiring people who only KNOW THIS culture.

If the author is reading this, there is one solution I have encountered: diversity. If you only hire white dudes in hoodies with stickers on their macbooks you're only going to get the toxic gamergate crowd, which has not gone away at all.

Believe or not there are other kinds of programmers in the world, and the most enjoyable products I've worked on were ones that had more women and more POC on their teams. Toxic asshattery can be minimized by not giving them complete control over the culture of a workplace.

I don't think diversity of skin color and gender will solve this problem in its entirety too. People of every race and every gender have the capacity to be this kind of self-centered egotist. People of every race and every gender have the capacity to be excellent developers who are focused outwards, on developing their teammates and building things well. Diversity is part of the picture, yes, but it's no instant win, either.

> People of every race and every gender have the capacity to be this kind of self-centered egotist

He didn't imply that women and non-whites can't be self-centered egotists, only that they're less so than white men. Despite the author of the article being Russian, a very different culture than the US.

A practice I've become interested in but don't have an opportunity to engage with(doing the solo ISV thing) is "mob programming", since it confronts a few axes of the toxic-developer problem:

* There is some built-in diversity of skillsets, if not demographics, by pushing team communication into a continuous meeting format where non-developers are given space.

* It forces some vulnerability into the mix, which gets you into a less inhibited state: "I don't know, but" is way more common if you literally can't run off and prepare some slick answer to every question or hide behind your ownership of the solution space.

* The I/O bottleneck of having a whole crowd at one screen moves the emphasis away from the lines of code, and towards the broader parts of problem solving and getting feedback. Everyone that's experienced always says that it's not how fast you type that matters.

* Feedback becomes less punishing. Everyone gets a chance to drive, make some minor errors, and immediately correct them, which keeps everyone on the same level and encourages a healthy attitude to learning, vs the anxious/punishing "all-seeing-eye judgment" nature of batch code reviews.

But mostly, I like the idea of a hypothetical mind-melded "superdeveloper" emerging from a mob - a coder that pumps out extremely high quality code solving exactly the right problems in a single iteration, without breaking a sweat. I do think I've seen it in bursts in the past, just not in a systematic fashion. My experiences with pair programming definitely suggest that it adds intensity to problem solving that isn't there alone, and it makes me suspect that we may just straight-up be "doing software wrong" by focusing on quantities of code edits and not the overall communication flows.

> If you only hire white dudes in hoodies with stickers on their macbooks you're only going to get the toxic gamergate crowd, which has not gone away at all.

Um, excuse me? I'm a white dude who wears a hoodie and has a sticker on my macbook, and I am not in "the toxic gamergate crowd." If anything you're the one being toxic.

The problems don't arise with individuals, but with groups. If you create a monoculture, it will be hostile to others that are not part of it.

Oh lord, the stickers. I felt an interview with a local startup turn south when I pulled my Thinkpad out and was asked "where are the stickers?".

You do want to express yourself, don't you?

(Sarcasm, Office Space reference, I'm with you on the stickers.)

Now I'm waiting for the next season of Silicon Valley to 100% repeat the "flair" conversation but motioning towards a laptop instead of a uniform :)

Agreed. I'd much rather not advertise for anyone/company, and especially not throwing sticky residue all over my expensive laptop.

In addition to a shared aversion to voluntarily turning my possessions into billboards, I'd like to add there's real value in keeping computing devices generic looking.

By plastering your laptop with a completely unique combination and placement of stickers one can discover in videos or photographs online from conferences or talks for example, you make it a whole lot easier to pick out your unattended machine from a set, a hotel room, or luggage.

This can easily be leveraged by assisting targeted theft, destruction, or sophisticated physical access attacks.

> In addition to a shared aversion to voluntarily turning my possessions into billboards

I know that feel, but you can flair up with whimsy rather than corporate tribalism. For example, I have a strawberry from the game Celeste over my MBP's apple logo. It's even less billboardy than before!

> you make it a whole lot easier to pick out your unattended machine from a set, a hotel room, or luggage.

Laptops get mixed up all of the time in security when you fly. A sticker both reduces the probability of this happening accidentally and also makes it harder for a thief to claim innocence if you keep your eye on your laptop and notice someone nab it.

I agree with you that one part of the solution is to stop hiring people who only know that culture. The ability of individual agents to sow dysfunction far outstrips any individual contributions, in my experience.

However, I'm not sure that diversity is really a solution so much as a symptom of a solution, which is a healthy professional culture. Adding women and POC may work tactically but it's not a great long term strategy for solving that particularly problem -- it's entirely possible for women and POC to behave in this manner (although it's quite a bit rarer in my experience). Not saying you're suggesting this, but I've seen quite a few orgs where women and POC are hired as tokens and don't get to real positions of power where they have the power and responsibility to truly run things and reform/refactor systems. Reforming your organization so that it's not toxic to women and POC is an indicator that you're not going in the wrong direction, but it's hardly the end goal. In fact, it should be positively mundane, boring, average and the norm (and hopefully, will become that way soon).

Will these problems cease at that point? I doubt it. Callous, abusive leaders exist everywhere, and short of a massive societal shift where nonviolent communication becomes required reading for managers and leaders (which is verbatim what Satya Nadella did at Microsoft to great effect), the high leverage move would be to start there and not paper over the root.

These problems come from the top -- corporate and engineering leadership. If you have a culture that accepts and allows for brilliant jerks, there are a couple root causes:

1) Your leadership aids and abets it

2) Your leadership is apathetic about it

3) Your leadership dislikes it but feels powerless to stop it

If you've got issue class 1 or 2, it's likely more pragmatic to leave than try to wage a cultural coup (of course, more power to you if you can pull that off). If you've got issue class 3, well, maybe you've got some options. You can convince leadership that they've got a certain kind of problem -- easy enough. Then you've got to convince leadership that a given approach could ameliorate it -- doable, but tricky and not at all a guaranteed success. Finally, you've got the hardest part: actually implementing it. The resulting shift in power could result in folks trying to sabotage things, and success will be doomed unless it is unilaterally supported and individually guided through by leadership. This is rare, but possible (again the Nadella reference). But again, the goal should be to create a cohesive, respectful, supportive teamwork oriented environment, and the means should support that.

If you can pull that off, achieving not just diversity but a safe, sustainable place for folks of diverse backgrounds to thrive becomes a real possibility. I think that's why it's worth setting our sights there in the first place.

As a professional, I got tired of being mean, so I switched to a different review style where I ask questions instead. I would still ask a lot of questions.

Then I started to worry that I was coming across as passive-aggressive, that these suggestions would be taken as veiled demands to fix things.

As a countermeasure, sometimes I'd explicitly say things like, "you can submit this as-is, but here are a bunch of things you might want to think about." This works ok for stylistic changes or things that aren't actual bugs.

That's not always realistic. Another trick that I think maybe we should do more in the industry is what I'd call ping-pong code review: When you get a review, either submit it if it's good enough, or rewrite it and send it back to them for review. Then they can either submit it, or do another rewrite and send it back to you.

This helps the most if there extensive changes needed, and you are a picky person who likes to have things just right. But, if it goes against company culture, you might need to make sure that people understand what you're doing and everyone gets proper credit for helping with the patch.

You don't think that rewriting someone else's patch would be considered rather arrogant? I've worked with a guy who's attitude to most things was "OK, let's submit this for now, I'll rewrite this later". I found that to be rather demoralizing, why would anyone bother to put in their best work when it's just not going to be good enough for this "god of programming".

I agree with the approach of asking questions and making comments like "some things to consider with this approach".

Generally speaking I find that code reviews are invaluable, but it takes some practice and good language skills to find the right "style" and wording in order to appear constructive vs. bossy know it all.

Yes, I agree that there is a risk of offending people this way and you need to put some effort into smoothing it over. Programming is often a people problem.

I've only done it a few times where I was the project owner, and I apologized in advance for being overly picky. There was also good reason for it since we were many timezones apart.

That's the socratic method. Far more constructive.

It depends what you're comparing to. Doing a lot of round trips during code review is extremely costly, particularly when you're separated by many time zones and you only get one round trip a day at best. Getting a patch in can take many days.

Telling someone to change the code (and maybe having them misunderstand) is often less efficient than changing it yourself, due to the extra round trips.

If you're sitting right next to each other, some other technique like pair programming is going to be a lot faster.

You bring up a good point about changing the code. I wouldn't want the reviewer changing the code himself and not getting reviewed again. But even if it was reviewed again, I would still find it problematic as the reviewer might make changes that are unnecessary and detrimental to the code base based solely on his opinions. For example, he prefers to use front end templates vs. backend or prefers to break up classes that should stay together, etc. Most code review issues that come up are a matter of opinion and it should be up to the author to change or not to change the code. If the reviewer feels so strongly about a change, he should submit a different PR in addition and it should be evaluated separately. I can't think of anything more enraging than coming in and seeing your reviewer rewriting your perfectly good PR simply because he wants to do it his way. At that point, I wonder why I even wrote a PR in the first place. Why not have this asshole write all the code then?

Yeah, so in the end it's all about the relationship between the programmers, and as I've said, this can be tricky. If you neglect that it's not going to work.

For example, if you're sending a patch to the owner of the code and they are known to be very picky, maybe it won't be considered an insult to rewrite it, since in the end it's their project and it saves a lot of back and forth? Or maybe it's a team where they have gotten used to it?

Believe it or not, starting from a patch that actually works is actually pretty helpful and does save time compared to starting from scratch.

I'm OK with sending a patch or pull request back to the author in any circumstance and using the author's work as a base for said patch/PR makes complete sense to me. The problem I've seen is when the reviewer takes it upon himself to make these changes directly in the current PR without giving the original author a chance to change or defend his code. This is exacerbated when said changes take an entire day or two of the reviewer's time and are so extensive that the original work submitted becomes irrelevant. This wastes the reviewer's time which could be spent on other things instead of rewriting something that already works and has been done and it wastes the author's time because it essentially discards his work. You're right, it is definitely about the relationship between programmers, but I think some ground rules would help in cases where the relationship is unclear or the programmers work async remotely.

> An alpha male with a huge stick.

No, you want to believe you're an alpha male. If you have to keep proving it every day it's not true.

You're acting out because you're insecure as hell and you're trying to prove to yourself that you're an "alpha male". A big part of being secure in your own opinions is letting other people be wrong sometimes.

At some point nothing you can say will stop them. Now you have to switch over to contingency plans. If you're absolutely sure it's going to break if they do it that way, then you need something more constructive than 'I told you so'. People trust you when you pull their fat out of the fire multiple times. Not when you dick wave at them.

> and in a way that I contemplated quitting the industry. I was too dumb for all this.

No, you're not too dumb. You're just wound tighter than a kettle drum. What you need, and I'm being sincere here, is a therapist, not another book on code styles.

We all have our histamine reactions to different things. While you're worried about 5 things someone else is worried about 3 others, one of which you never even thought of. I would hate to be on a team where everyone only cared about exactly the things I care about. My priorities help find issues faster but I don't find them all myself. A team of me would be almost as lopsided as the things I push back against. I try to remind myself that when I'm having trouble with feeling like my #1 priority is someone else's #15 or NaN.

We had a guy at a previous job who was a toxic douchebag, not radically different than the guy describing himself here (doing F# too, actually, which I found to be a funny coincidence).

The guy was undeniably brilliant, pretty clearly smarter than anyone else on the team (including me), but he had a way of bringing down the entire team to a point where it actually felt like we were less productive a result. After about a year of this, he was fired.

Ironically after he was fired he and I became pretty good friends, and I feel that, at some level, I became the new toxic douchebag with my (probably unearned) feelings of superiority. When I realized this, I actively tried to employ a bit more empathy when looking at code reviews and dealing with people that I felt were stupid, since I was the "stupid" one before.

I often discuss with people the philosophical question of "is it better to have someone who's really smart but also an asshole, or someone very nice who's barely competent?". I tend to lean towards the former, but I can't say for sure.

It's one of the things I'm most pleased about at my current gig that reviews are in general collegial rather than adversarial.

I wish I had a magic blog-post-length formula for how that happened. I don't. I know I personally talk about it in interviews, and we hire for developers who think about development as a team sport rather than a solo FPS rampage, and we talk about code review sort of a lot in onboarding.

The way I explain it to noobs is that a collegial review culture is a question of trust. When we all believe that we are all on the same team with the same goals, we feel safe, and we can trust that (1) critical feedback we receive from others is intended just to improve the code, not to score points on us personally, and (2) if we give feedback that is critical, it will be received in that spirit as well. And it's our responsibility to give critical feedback where the code under review doesn't meet our standards of efficiency, readability, and maintainability.

Grandstanding or mean-spirited reviews break down the team-wide spirit of trust and the feeling of safety; they are a far greater danger to the integrity of the code base than a badly-coded method or spotty test suite.

I've seen a few cases where otherwise talented developers would kind of miss the point of code reviews and focus on code style much more than the code itself, nitpicky stuff like sorting of imports, etc., leaving hundreds of comments while at the same time overlooking quite serious bugs.

Presumably, codestyle comes easy for them due to their neurotype, but they have a hard time reining themselves in and just end up wasting thousands of other developers' hours for zero business value.

I've never understood why these people don't just spend a few minutes adding some rules to ESLint or whatever.

However, while this person really feels like an asshole, it's incredibly mature of him to recognize his past behavior and try to improve.

For many developers, it's incredibly satisfying to "be right", and due to the fact that's it's easy to leave some comments, the humanity of their coworkers can often take a backseat.

Step number 1 for any dev joining my org; learn the difference between "how I would have done it" and genuine optimisation/bugs.

Its probably too carte blanche; but ive reached the point where I will refuse to rule on codestyle issues (general answer; whoever did it first sets the style)

When I was a student I had this same experience peer-editing prose. In reading a classmate's paper, I found endless nits to pick about syntax and minute points of diction or style, but much less to say about the actual argument. The main reason for this is just that it was easier. It doesn't take much effort to call out bits that strike one as unaesthetic, but evaluating the substance of a paper requires that I really think about what it's saying.

Consistency of code style does matter, up to a point, to people reading and understanding the code. As you suggest, setting up ground rules and tooling helps

Code authors who don't see the value in consistency of code are potentially a problem - if authors are submitting code reviews with hundreds of actual style issues, that's either a failure of process or the author to write readable code (not sure if that's what happened in your examples, to be clear).

It could also be a testament of the reviewer's inability to read code written differently.

Style is not consistent across authors, codebases, projects, even companies. I had to learn to read code in many different styles.

Enforcing consistency can be done with formatters/linters. It doesn't need to come up in review.

It depends on how important you think it is to optimize for future readers or maintainers of the code. I weigh that pretty heavily and a do think relatively minor style issues impose a tax once they're pervasive in a codebase.

Tooling is great, but I think there are a lot of style issues that aren't readily enforceable with linters - commenting, naming, control flow, abstraction, etc.

I sometimes fall into a similar trap. Usually on the larger changelists where I was struggling to keep the focus necessary to find the serious bugs, but not being comfortable only looking at half the files in the code review - feeling like I'd not be doing my part of the job I didn't at least look at them. I have to remind myself that there's a limit to how useful nitpicking can be, and that it can become counterproductive as well.

Sometimes the quite serious bugs are just hard to notice, as well. I recently reviewed a large changelist dealing with lots of multithreading and locks, and cases where locks aren't taken to avoid deadlocks. I have 0 confidence I caught all edge cases, which terrifies me for multi-threaded code.

I don't have your context, but if the changelist author is writing multithreaded code where there's no way that a reader can convince themselves of the correctness, then my heuristic is that the burden is on the author to improve the code to be easier to reason about and add enough tests to exercise all of the interesting code paths. There are always techniques to simplify code or make it easier to grasp.

I have similar heuristics. I'm drawn to Rust because I really like the idea of having to prove it's at least data race free to the compiler (not that this will solve the problem of deadlocks.) In most situations I'd push back on code that got half as tricky with it's multithreading.

Simplifying is easy. Early versions of this code, years ago, were simple. They weren't even multithreaded! Just a simple implementation to unblock other devs.

But now it's a highly used bottleneck that must be high throughput, low latency, support asynchronous cancellation of requests, interacts with the main thread for third party APIs that aren't thread safe (such as d3d9), interacts with the main thread for our own APIs which aren't thread safe by design (our debug replay system replays the events of the main thread), but must avoid synchronizing with the main thread for performance elsewhere...

Simplifying this without performance or feature regressions is significantly more difficult. Possible, but difficult. And benefits from slow, testable, incremental changes. But that means reviewing incremental diffs on the large and complicated existing system in the interim. At least it has some of our best test coverage, including lots of tests to help try and tease out threading bugs. They don't always succeed at that, but they do help.

I am an notoriously tough code reviewer. Not because I’m mean, but I’m thorough and consistent. I’m never passive aggressive and simply factual. But newer team members can find dozens of comments until they understand what my expectations are.

I can generally differentiate between personal opinion and bad form. I rarely comment on things like variable names, or function names unless they are really confusing or misleading. I have my own style, but the existing style of the code should be what we code based on, not personal opinions.

I think the OP is shirking their responsibilities for what they’re being paid for. If they are doing a code review it’s for the benefit of the team, so that the team member understands what their expectations are. You don’t have to be mean to be a good code reviewer.

I agree with you. It's possible to work together with the person who wrote the PR to make the code as good as it can possibly be, while also not being a jerk. I try to be very thorough and offer lots of feedback, but also approach things from a position of respect. For example, instead of "why the hell would anyone do it this way" I instead think like "maybe they thought of something that I haven't" and I hope that it comes through in my comments.

When I submit my code for review, I don't just want a rubber stamp, I want thoughtful feedback. If something can be improved, I want to know about it so that I can do it better next time. I try to pay other people the same courtesy. Another commenter said it's all about "disconnecting ego from the work" but I don't really agree. I care a whole lot about my code and that's why I take code reviews seriously. But that doesn't need to translate to anger or feeling emotionally devastated as the article's author somewhat bewilderingly seems to ascribe to his "victims."

Another thing that I keep in mind is a joke I heard one time that you measure how good a piece of code is by the WTFs per minute exclaimed when reading it. It's a metric, not an emotional reaction, and it's going to be at least unity no matter how good the code is.

Then again, I've had the good fortune to work with good people, so maybe I'd be seeing red too if my coworkers were writing really awful code all of the time.

I can recognize some of the traits from the article in myself and I can guarantee you that I (and, I suspect OP as well) would be quick to claim that my comments are "merely factual" and that I am great at differentiating personal opinion from bad form as well.

I don't know you, so you might be 100% right but if the article makes you feel slightly defensive, perhaps it's a good reason to do some introspection.

I don’t feel defensive at all, I think OP is probably a terrible code reviewer and makes me feel better about how I code review.

Good developers rip bad code the shreds.

Great developers see the opportunity to grow someone.

The former shows ownership of the code, but is generally bad for a business.

The latter understands that a successful business is more than simply his competence.

Reaching this level of realisation is critical for being a good lead; I interview loads of clearly solid programmers who havent figured this out, and sadly price themselves out of the market (I blame their previous managers for missing their growth opportunity)

It bothers me that the conclusion doesn't escape the framework of code reviews as places of conflict, and that the author feels guilty for his claimed competence.

> No big deal if the code’s not good, I can fix it myself it I need to.

A mentor can't be passive like that. Don't leave 200 comments like you used to, but distill them into something smaller, more cogent and digestible. More importantly, drop the idea of "good" and focus on what the implementor was thinking. I think the author understands the need for more effective communication in code reviews, but unfairly reduces the exchange to an "argument."

It's also dangerous to correlate work-life balance with incompetence. It's not a contradiction to be good at your job, current with the industry, and present for your family.

The guy appears to come to the realization that he causes more conflict than necessary.

The purpose of code reviews is to get quality code. Quality code makes the project work. But why do we work on projects to begin with? To benefit people.

This guy is realizing the conflict between the two goals of getting quality code but also serving people.

People need criticism to improve, but it needs to be constructive, otherwise the fragile human psyche inside of every person may not recover.

Ancillary point: there was too much coding standards as tribal knowledge in this, and a lot of cases.

Many times developers get held to standards that they didn’t know existed. This still triggers developers to feel bad about their work (or themselves). It’s usually after someone has left that they find out some ‘standard’ only exists for that particular company.

It’s hard to make global standards, but company standards are easily do-able without having to endlessly specify every coding situation. What the standard doesn’t cover should be addressed in code review or before.

Sometimes languages come with standards (C# coding standards are better than JavaScript standards, for example) and those can be a good proxy or starting point.

The problem with the author is that he expects all code to be written the same way that he would have written it.

Nobody can read his mind but yet he's expecting rookies to write like veterans.

Typically experienced developers mentor junior developers. If that's not what the author wants to do, then hire only senior developers to not worry about junior mistakes.

Also instead of pointing out all the wrong things, pointing out the good things too are a way to boost the junior developer's motivation.

The overarching problem is a lack of empathy. In my decades on this planet, I've seen it over and over again in interpersonal relations under all circumstances, but nowhere near as often as in the realm of text communication.

Humans need face to face contact, because this kind of contact gives you instant feedback about what effect your words and actions are having. It's how most people experience empathy.

I'm one of those who is far better at text communication than face to face due to the luxury of being able to carefully consider my words and their effect rather than having to clumsily do that in real time, but a side effect of this is that my every word gets rewritten multiple times as I try to get the overall tone right, and try to head off potential misunderstandings. My success rate isn't super great, but it's far better than my face to face.

With most people, the opposite seems to be true. Worse: they seem unaware of text communication's shortcomings in terms of the social and emotional feedback loop - empathy. The most toxic people online are often an absolute delight in person, completely oblivious to the damage they're doing because they're empathetically blinded in the other medium, and unaware of the fact.

The author sounds like somebody I probably wouldn't want to work with. They seem to have an inflated view of their own ability. Did he consider pairing with the person or trying to mentor them? Russian culture is no excuse for being a dick, be the person you want to be and treat others how you would like to be treated yourself.

As long as the code does what it is supposed to do with acceptable performance then I don't comment and approve the PR.

Because unlike people that think there are right ways I know that it's all religious arguments. Last I checked there are 0 credible reproduced scientific studies about what is the correct way to program. Or even a better way. So as far as I'm concerned it is all opinion based and sure I also have my opinions but, consistency, rule of least power, DRY, agile, OOP, single responsibility principle etc etc are all religious notions.

There are only 2 things that matter: does it work? does it work fast enough?

And normally the arguments boil down to: "Oh but it is not changeable!" - maybe you should make something new "I can't read it!" - I also can't read Chinese "I don't like it" - well not everyone likes chocolate

I agree with you, a code review isn't meant to get the best code ever, mostly because as you said, there's no universal correct way, but it's meant to make sure the code change is doing what it's means to be doing.

What is not clear in your comment though is that that means looking at the code and making sure there's no mistake too in it too.

It may seems like it's working as expected, but maybe you'll find an edge case that was missed by the original developer.

It's possible too that the solution of the original developer was clear to him, or that it's clear knowing the context of the ticket, but once in the wild, that code may become incomprehensible (which later on can bring more error because someone misunderstood why that line was there and changed it incorrectly for example). That can be easily fixed by adding a tiny comment there.

Sure doing what it's supposed to do in an acceptable performance is great, but I think there's a bit more into a code review and ignoring theses can be quite problematic.

> "I can't read it!" - I also can't read Chinese

Doesn't this show there's an issue though in the long term? If you are the only one that can read Chinese in the company, maybe it would be a better idea to consider using English?

In most cases, designing for maintainability and reducing the likelihood of future bugs is more important than maximizing speed.

Or to put it another way, the human costs of maintenance and bugfixing are usually large, and may even be the largest costs on a project.

In practice there are things that are not religious arguments nor problems of technical correctness.

For example, in a recent code review I found a function where an argument was being passed in but not used, and a constant of the same type with a similar name was used in the body of the function. That obviously happened because the author got halfway through refactoring their code and didn't realise, and we were both happy to fix it.

"If a guy brings me his code, and it has mistakes, it brings insane pleasure from how smart I feel ... And if you tell me that you haven’t had this feeling ever, then you’re lying."

Wrong on so many levels. Taking pleasure from someone else's mistakes is a sign that something is wrong with you. That is all. The only thing that is exceptional is that this industry has a high tolerance for this behaviour.

It also sounds like this guy is destroying value. If you are such a poor manager that you try to get rid of someone rather than bringing them up then you are a net negative. Again, I have come across this in other industries: the manager will go before the junior. No question. Writing bad code is nothing compared to the impact on culture of one bad apple.

The secret to good code review is addressing the code, not the person. Ask questions and make suggestions, rather than being critical of the other person.

Instead of "Why would you ever do it this way?", try "I think I would probably do this like (example)"

Also, be liberal with complementary reviews, as well. I see a lot of pull requests where the solid, workday, well written code is just passed over, or given a ":+1:", even though it's definitely worthy of praise.

You wrote a new class extracted from an old confusing method, it has good tests, and the feature is working as requested? A triumph.

I strongly disagree with his conclusion, even if I haven't nor ever would be the kind of guy who would just be condescending during a review. No, every mistake during a review must be commented. I just do it politely, as in, point out the error and the possible fix and leave it at that, without any implication about the developer's skills or lack thereof.

That said:

> It can’t be open-sourced or used to lure new developers.

This is a mostly unrelated takeaway that I wish more companies followed so that I could take a look at what I'd be working with when they try to recruit me.

> I haven’t ever thought about the issue seriously and didn’t have any decent arguments, but I couldn’t stop arguing nonetheless. I had one goal — to win and save face. I still don’t know why.

I'm 100% like this. I'm not proud of it, and while I guess I can hypothesize about "why", my takeaway is the same as the author's : it's a horrible and potentially very hurtful anti-pattern.

Kudos to this dude for raising his hand and acknowledging his own problem. All I can say is, I'm the same way and I can and should do better.

Here's one guiding principle I've thought a lot (and learned a lot about) during the past few years: learn to draw the line between how you allocate communications on the pull request and communications over chat -- don't be afraid to have a bias towards the latter. PR comments should be fairly light on a per line basis, and you may want to bring up architectural concerns if there are any, so that they're recorded there for posterity's sake.

But you want to look at it like an artifact that you'll later on refer to. Don't be a pedant and ding based on style. If there are substantial design flaws, politely note your concerns, and then hash it out over chat or in person. Your goal is constructive collaboration, and it's really important to use the right collaboration tool for the right job.

Often times, what happens in code reviews is that we find up front design that we neglected to do, or which was done in a way that has structural problems. That's fine -- in fact, the ability to do that incrementally is part of why lean works well. But that means that you've got to then do a proper design session (formal or informal) to get to a better ending place. If you really want to be cooking with gas, figure out a lightweight way to structure and record the results of these design sessions so that people will use and refer to them.

I've seen teams grind to a screeching halt over poor usage of these processes, and not because any of the engineers were poor individual contributors!

I couldn't really read it fully kind of in a run. But it seems like that the author isn't just talking about the language but that commenting on every bad issue of the code is bad? I don't agree with that.

I have a done a lot of review not as much lately but my principle is pretty simple - Can this be made better to the best of my understanding and knowledge. I will give it the same attention as I will my code. When my first version of a code or the POC is done I will think of what can be improved upon with the goal of the final version to be cleaner, faster and more robust if possible. My job is to help the author and the company to make sure together we get the best version possible merged of course with an understanding of cost-benefit.

The language is of course important but not catching issues on every line if that's what I find. I will appreciate if someone does the same thing for me. At one point I have reviewed more than 50% of the codes in my company and I know how hard it can be to do so with full care so I take it as a favor when somebody does a good review on my code finds mistakes or potential improvement. I have had way junior developers giving me feedback ranging from better names to serious bugs. I have also always explained why a certain idea should be explored or might be better with the author regardless of how junior they might be. I have always liked the developers best who leaves the ego out of it both a reviewer and author of a certain piece of code. Pride and Ego I find are unrelated. I take pride in giving my best both when writing a code or reviewing one all the while knowing not just that there are other developers much better than me but even someone who is on average worse than me can still find potential improvements in my code. I like to think that I can take any comment on its merit and not my perception of the person.

Well, all I can say is I'm glad my team of developers is not like this. Though there was one code review from my senior engineer going increasingly heavy on asking for documentation examples on a specific function. I felt like he was angry with me but maybe he just had a strong feeling those examples were important... Actually I will never know so time to forget it :)

I had a friend in elementary school that was a lot like this. As we stayed friends all throughout our school days, he would constantly argue with me about the stupidest things. Even when I was right and could demonstrate he was wrong, he'd still try to win. Ironically, we're still good friends because I learned to look past this BS and not care.

I’ve had to go through code reviews (as a freelancer) conducted by people who clearly were angry people and who had no (current) development experience. Their job titles were Technical Design Architects and part of their job was to make sure outsiders weren’t polluting their companies code base.

When I’d justify my decisions they’d go Google and always manage to find a counter point. But their having to Google it made it obvious they lacked deep understanding of much of anything.

There was one guy who was particularly notorious for being harsh and awkward to work with. I found this to be true but he was also the only guy who seemed to understand concepts so I was able to work him (to everyone’s surprise!)

I saw many other developers frequently get a bruising but I always managed to take a step back and focus around the real problem - the people in charge were just angry.

And this is why we hire for empathy, and pair-program instead of doing code reviews. Code reviews and features branches don't cause this type of behaviour, but they are a fertile field for this to flourish.

I'd rather have ten mediocre developers who can be part of a team than one gifted diva.

Good on you.

"Greater in battle than the man who would conquer a thousand-thousand men, is he who would conquer just one — himself. Not even a God, an Angel, Mara or Brahma can turn into defeat the victory of a person who is self-subdued and ever restrained in conduct." -- Dhammapada 103-105

Interesting that he did all this, and did F# in his free time. The F# community has been almost 100% positive and welcoming in my experience, which is impressive given how many people come to it with no functional experience and so make a lot of novice mistakes.

I do a lot of code reviewing, and I still have to use the backspace key sometimes when I find myself oversuggesting. I have found code reviews (when done well) to be a fantastic teaching tool. I also think that we get some great value out of code review by less experienced folks too. Those developers without decades of knowledge sediment are more willing to say, "I'm sure this block is awesome but I have read it 10 times and I still don't understand what it does." That turns into one of two things: an opportunity to educate the reviewer, or a sign to the author that the code is too complex (i.e. less peer-maintainable).

I enjoyed the post.

My feeling is that criticism, no matter what the domain, should be relatively dispassionate (neutral in tone), and come from a desire to help and with a dose of humility. Of course, I sometimes fall far short of that ideal.

I actually feel that criticism should be empathetic rather than dispassionate... it should be worded to show that you understand the person is a competent professional who was working in good faith, but might have missed something or made a mistake.

This is why I try to word reviews as questions, not statements of facts:

"Is this supposed to be reversed? I think it might not do what you want it to"

"Having a bit of trouble following this logic.. is there a way to make it more clear to future readers?"

"I might be missing something, but I don't see how this code can be reached?"

Don't give the review in such a way that implies you know everything and the other person was wrong. It should be a dialog, and be based on the assumption that we don't yet know the right answer and are working together to figure it out.

Reminds me of a lot of code reviews from my Google days. There was even a formal rite of passage called "readability" where you went from being rightfully humiliated to rightfully humiliating others.

Oof, talk about burying the lede:

My personality today isn’t my disease. It’s a disease of the whole industry, at least in Russia. Our mentality is predicated on the cult of power and superiority. And that’s what we need to fix: just stop being that. It’s quite easy, actually.

A bit of irony in the, "It’s quite easy, actually," given the thesis. I can't imagine a harder talent to learn than emotional intelligence. I would rather teach someone...I dunno...concurrent programming than how to stop being an asshole.

Great article. I suspect most that have gained a level of proficiency have felt that pull and acted on it to some degree. The best thing for you now would be to be hopelessly crushed by someone far more capable. Or get fired for being an asshat at a vulnerable time in your life. Then you may achieve the humility and confidence to become a true leader, which requires you to allow others to feel strong.

Being tough on people seems to work if the knife isn't aimed at yourself. Eventually, it will be.

People that are really good at things are graceful and helpful usually.

PS-Question: Author mentions Russian culture being an influence. I feel like he's saying there is an emphasis/high value on ultra-competence ultra-stoic unshakability sort of state of being? The meme is that Russians are intense, which I fully respect.

I have been on both sides of the table, maybe on the side receiving abuse more often, but yeah I should stop being so hard on people with code reviews. The thing is if I'm not hard on them someone above is going to come down and question me about it. Maybe just talking to the dev outside of the code review so they can improve without leaving a million comments would help.

For this reason at my company we often do the code review of a junior developer's first pull request in person and in private. I think it's worked decently well to ensure that comments can be the start of a conversation and not a reprimand, and to help the junior to understand that the comments are not meant to be a personal criticism.

BTW does anyone have a good guide, book, tutorial on doing code reviews? Everywhere I've been its kinda adhoc with no rules or guidelines.

Often code reviews come in after someone has put a few weeks of work into something and its too late to change how it was done. Or a review is passed in by someone in a hurry without actually critiquing it.

We had a senior engineer (who sadly moved on to a different company) periodically give a talk to new hires about good code review practices to set expectations and ground rules. Some of the key points:

* Authors are expected optimize for readability of code, even if it takes longer to get the code in.

* Reviewers should generally only start reviewing code if they are ready and willing to continue the review to completion (i.e. no drive-by reviews). Once they start reviewing they should try to minimize response time.

* Focus the reviews on testing, correctness, readability

* Consistent code style is important (see optimizing for readability)

* Break code reviews into the smallest reviewable units. Often these are somewhat large because parts of the change don't make sense in isolation

* The code author is responsible for writing code in a way that it convinces the reviewer that it is correct (see optimizing for readability)

* You should approach it collaboratively - you're working together to get the work done and make the codebase as maintainable as possible

* You should aim to leave each bit of the codebase that you touch in at least a good a state as it was previously

my one suggestion is to make your code reviews have more questions than statements. so instead of: ‘forgot to check for null here’, do something like: ’do you think we can have a null issue here?’. in my mind, this word change makes the reviewee feel like they’re not just being told what to change, but actually have a sense of code ownership and control over their work

Somewhat related, can anyone recommend a place to get code reviews. I work for myself, and I haven't had many opportunities for code review in my career. I'm willing to pay market rate for a senior engineer's time. I personally need feedback for C, Swift, Python, and Javascript.

Sadly, developers do it for free. https://codereview.stackexchange.com/ Pay with market rate upvotes. :)

Code review is a power tool that improves codebase and improves software developers' skills.

If code review breaks software developer's psyche, then this software developer is unlikely to be any good in the first place. How can you be good without ability to listen to code review feedback?

Code reviews aren't supposed to be humiliating. This article does a lot to illustrate how a negative attitude in review comments can be counter-productive, but the conclusion feels like code reviews that point out dozens of flaws in the change are bad.

I disagree that pointing out every flaw in a code review is a bad thing to do. I believe that these could be positive experiences with the right attitude from both the reviewer, the submitter, and the team's management. A few things that really bugged me while reading this:

- The reviewer feels like pointing out a flaw is an adversarial zero-sum sort of action that affirms his superiority over the submitter.

- The submitter will feel bad about each feedback given (at least in the POV of the author). If the feedback is mean-spirited, then sure, but if it's constructive, then this shouldn't be the case.

- His team fired the developer who received "too much" feedback. I'm sure there's missing context here, but if that's the main reason, then this is pretty messed up.

Finally, his conclusion to not submit the review really bugged me. I do agree that no review is better than a mean-spirited destructuve review, but IMO this is still a failure on the part of the reviewer. He should write the review in a constructive manner and work with the submitter to hash out the issues. If it's an argument, then fine, work through it... but teammates should help each other grow and be better.

In this case, by not submitting honest and comprehensive reviews, I believe his team and his product suffers in the following ways:

- The reviewer has to waste his time "cleaning up" the code later on.

- The submitter loses out on the knowledge transfer that takes place during a good code review.

- The product potentially has unfixed bugs.

- A fear of conflict is further instilled in the team, and criticism now becomes off-limits because it is considered destructive due to the negative attitudes of everyone involved. This further hurts product quality, and the result is that everyone codes in isolation out of their ivory towers instead of collaborating on a product together, among many other detrimental effects.

I think the solution at least begins with:

- A better team commitment to personal growth, and a sense of responsibility on the part of everyone to help out their teammates in this respect.

- Clear team guidelines on lint/style standards to eliminate the need to argue about it.

- An attitude that feedback is about the code and NOT about the person submitting it. Everyone should feel like an owner of the code, and all discussion should be about the code and how to improve it.

- Comprehensive code reviews. Note everything you find. Maybe call an in-person review if you feel like there's too much to comment on.

edit: typos

I suspect code review comments could be treated as a form of creative writing, and good (modern) writers are emphatic to their reader. All you have to do is read the comments as if they were addressed to you, before hitting that submit button.

>imagined how I could’ve solved it, and looked at the code. As always, it was pure rubbish. Nowhere near the solution I could’ve come up with.

I hate this. This always shows a developer whose only idea of a good developer is himself.

I thank the author of this article - this happens all too often in the industry where it starts sliding into dealing with massive egos rather then actual improving teammates, coaching and truly helping the codebase.

When did code reviews become so ubiquitous?

Articles complaining about the reality of code reviews seem to be commonplace. But if they're not working out, why not just abandon them, maybe try something different?

I'd argue that if you have a toxic team culture then no matter what you do things will escalate. Code reviews are a focal point but if they didn't exist then the focal point would move somewhere else. For example, engineer A commits some code and then engineer B rewrites it and then they get into an argument about it during a meeting. Toxicity in code reviews is at least auditable so a pro-active manager can nip it versus someone going off in a meeting they weren't part of (in which case it becomes a she said/he said situation).

I don't think "toxic team culture" is the only way code reviews get frustrating. Other issues include frustrating waiting for reviews to be completed or the flip-side of people being knocked out of flow to do a review because "reviews are top priority". Relatedly, arguments about what the appropriate size is for a reviewable change.

The one alternative that (some) code-review advocates seem willing to accept is XP-style full-time pair programming. But that leaves even less room for individuality and solo accomplishment.

The issue is that code reviews are hard on people and good on code bases.

Dollar for dollar, code reviews are more effective at finding and fixing bugs than any other activity we can do, including QA tests. However code reviews are very hard on people, and can easily create conflict.

Which one matters more, and how careful people are, varies widely by organization.

My recollection is that the studies showing that code reviews have high defect yield are for extremely strict methods. Everyone in a room, prescribed roles for each person, line by line, checklists, forms, the whole nine yards.

Comments on pull requests are not on the same level and those findings shouldn't be hastily generalised.

My impression is that comments on pull requests do not catch as many defects as formal review, but catch even more per dollar spent.

You could write this exact article but for the research peer-review process...

Actually, can somebody write that? :P

My review to this article.

Change your position to .NET (uppercase). There's no technology called .net (lowercase).

Thank you.

I believe it's the past. Professional IT culture in Russia is in many ways retrograde compared to Western countries, especially the part that concerns mentorship, education and professional ethics. It's changing, but slowly.

The approach that I've seen work is to have high standards for code, but communicate the expectations upfront and make sure that code reviews are grounded in those standards (for new people, this means over-communicating, but it becomes more intuitive once you've worked together for a bit). And whatever you do, be constructive, not rude.

If your codebase is going to be maintained for a long time, letting sloppy or inconsistent code in is going to make it much harder to maintain or evolve in future, so just lowering standards to make people feel better doesn't work. It's totally reasonable to hold people to a high standard if they're submitting code that others will have to maintain.

A lot of the problem is if people start off with the expectation that their code should be merged with minimal modifications, they're going to be upset when a code reviewer seems to be moving the goalposts on them. Also if the reviewers comments are arbitrary (or seem arbitrary) because they're not obviously grounded in consistent standards.

I've seen a project go through the process of tightening up code review standards and it was initially painful (a lot of people used to the old way feel like they can't move fast enough, etc) but ultimately worked out and resulted in people shipping working features at higher velocity with far less time spent on rework and maintaining overly-complex and buggy code.

There's also way less conflict in onboarding new developers because standards have been made clear in advance and reviewers are expected to be respectful and coach newcomers through the review process.

The hard part, I've found, is working with developers who can't or won't get code to the expected standard, even with coaching and detailed feedback. As in, repeatedly making the same simple mistakes pointed out by reviewers and failing to understand and address relatively straightforward feedback. I'm not sure if there's a solution to this, beyond minimizing the damage and trying to move the person to a role that they're more suited to. In one such instance the person was, in retrospect, a complete liability - they switched to a different team with looser code review, got a "bugfix" committed with some obvious errors that, if not luckily caught by our team, could have potentially had disastrous consequences for some customers, then left the company. Maybe the lesson is that high code review standards contain the damage from such people.

Edit: I also think this requires code reviewers to approach the reviews with the attitude of "what do we have to do to get this merged without compromising our standards?", not a desire to tear down the person or block the code change.

Author has a strange perspective of himself

an or condition in a constant; yeah you'll be the judge...

Are you implying there's something wrong with that?

it's a "constant" so it's value cannot be altered during execution of the application.

basically starting a rant about being a tightass about code review with a bad piece of code

This guy sounds like a massive, toxic asshole.

I'm tired of dealing with people like this. It does not have to be this hard. Tech companies should fire more people just for being jerks.

> I was mad that, while I spent my nights learning F#, my daughter started calling everyone around “fathers”.

Huh, you mean missing out on the good parts of life while martyring yourself for a company that does not give a shit about you can lead to resentment and toxic work culture? YOU. DON'T. SAY.

Honestly, a lot of the time management will stoke the "hero coders" ego so they will do the free work.

Management: Oh, Hero Protagonist! We need you! How will we ever earn enough to give our CEO the 15 million bonus if you do not martyr yourself for this feature?

Hero Protagonist: They're right. I am the best and smartest. I better dump all of my emotional labor onto my coworkers and significant others and bury myself into doing free work for my company.

> When you work as a developer, you always have to argue. You, as a team, arrive on a solution after a lot of argument, even though we call them “discussions”.

I do not feel this way at all. There are productive ways to have discussions that do not involve getting defensive and arguing.

The fact that you have to drag all of your discussions into toxic arguments in order to save face is... sad.

> And yet it’s somehow important than your arguments “win” more often than not, just to feel good and confident in your power.

No, it is not. That is just how you feel to justify your constant toxicity.

I see very little "power" in bullying all of the other developers on my team into following my opinions.

It sounds more to me like the author is trying to illustrate those toxic thought processes (based on his personal experience) using that second-person perspective, rather than actually claiming that those things are true. It's just the language barrier that makes it not work as well. Check out the last few paragraphs:

> This review I kicked off the article with? I didn’t send it. Instead I gave the guy a couple of comments and politely asked to fix a couple of things. No big deal if the code’s not good, I can fix it myself it I need to. But I can’t fix the psyche of a guy broken by dozens of harsh reviews.

> My personality today isn’t my disease. It’s a disease of the whole industry, at least in Russia. Our mentality is predicated on the cult of power and superiority. And that’s what we need to fix.

If the author is coming around by realizing the error in their ways; a way that seems painfully obvious to you, I don't think it does anyone any favors to chastise them for not knowing sooner. Its clear to say not everyone grows in environments that reinforce positive thinking.

Further, unless I am misreading the tone of the article, the author is listing out all of the defects you quoted not to defend or justify, but precisely to highlight their ridiculousness and falseness.

The sad part is he was probable middle aged by the time he realized this, those projects he worked on are probably dead.

> I see very little "power" in bullying all of the other developers on my team into following my opinions.

The author actually addresses that. It's just the style of the article takes you on a journey in his mind before getting to the conclusion. It's a different style of writing. From the article:

If we were being laughed at while young, it doesn’t mean you have to return the favor later on. The vicious cycle can easily be broken. Life becomes easier if you learn to lose arguments, if you can admit that another developer is more talented than you.

I think the author is trying to surface the conclusion you've made, but I'm not sure it merits such an emotional response from you.

The article is thought provoking. It makes good points, even if you don't like the author. I can relate to a lot of his examples.

I thought you were exaggerating but you're not; the guy sounds like a real pain to work with.

I also felt weird reading this. I really want to see some of this person's "code reviews." Two hundred comments on a pull request? How many of the comments are useful and actionable?

I love having my code thoroughly reviewed. It does not ruin my life to see someone say "I don't think this is a good idea." I think you have to be operating on a different level to be ruining lives through pull request comments.

The author is talking about a learning experience he had regarding a deficiency he identified (being a jerk) in his own work (reviewing other coders' code).

Meanwhile, your comment levies personal criticism toward him using pretty inflammatory language ("toxic," "sad," "asshole," "jerk," etc.), because of the way he responds to people who are learning.

Interesting juxtaposition, that.

>I'm tired of dealing with people like this. It does not have to be this hard. Tech companies should fire more people just for being jerks.

The problem is that the traits that make someone a 'jerk' are often the same traits that make a '10x developer'.

Did you finish the article?

This piece is so obviously written as satire / hyperbole that I'm tempted to think your response is as well?

I have met quite a large number of software developers in my day, and if I didn't recall meeting them face-to-face, I would describe the behavior of around 15-20% of them as "obvious satire".

The article finishes with the author explaining that (at least part of) the article is imagination.

This comment has bullying energy in its own right.

>And if you tell me that you haven’t had this feeling ever, then you’re lying. Tell me about higher goals, training rookies and all that — I know you’re simply too full of themselves. And if you try to tell me that you learned to defeat that feeling (however it manifests in you), then I must be a pink unicorn.

Are you kidding? Call me a liar if you want, but I don't feel that way when doing code reviews, and never have.

Yes, when I was a teenager I wielded my knowledge like a titan. The meager amount of knowledge I had.

But as a professional, I have never once been mean-spirited in a code review. Everything I send back is to keep the code clean and maintainable and to help the developer improve.

Not everyone feels the need to be "right" all the time. And by that, I mean being perceived as correct, even if they aren't. For me, that was a function of actual skill.

The better I got at programming, the less I needed everyone else to acknowledge how good I was.

Even better, that applied to the rest of my life, too. I haven't felt the need to show off my intelligence or skills in a long, long time. I simply do my thing and if/when someone notices, it's a great feeling. If they don't, I still enjoyed doing it well.

So you haven't -ever- been overly critical or tried to show off? Because your comment made it sound like you have improved over time and feel less of a need to improve yourself now.

In a code review? No. Not in all my years as a professional developer.

As a kid? I'm sure I did. I probably even did it about code on the internet. But I wasn't a professional then and wasn't employed as a developer.

My experience with having my code reviewed was overwhelming positive, if frustrating at times. There was one reviewer in particular who had a way of asking just the right questions to point the way to a better approach. It also changed the way I write code, too. Little things like naming stuff, internal consistency...I grew to have an intuition about these things.

I try to view code review as my chance to pass this on. It's about teaching someone how to read their own code with a critical mind. It's definitely not about superlatives like best, perfect, whatever.

> I was mad that, while I spent my nights learning F#, my daughter started calling everyone around “fathers”.

A lot of elitism comes from going through a high level of discipline for no reward.

Just do what is fulfilling

THIS.

Rich people collect money; find themselves empty inside; seek more money;

Find no reward in shredding through meaningless suffering; demand payback; be forced to extract blood from your fellow turnips.

wisdom

That's a really interesting idea, do you have any source?

This author sounds like he would be a poison pill on any team.

We recently managed to get rid of someone that wasa this sure of himself that he had the impression that he carried the team.

While his technical skill was sound (though no better than any halfway-decent senior developer) the reality was he was a poor communicator, kept knowledge to himself, posted snarky comments on reviews and cast apersions on everybody else's skills.

He was an egomaniac and the team has fared much better without him.

If you ever find yourself with a team member like the author of this blog post, be mindful of the damage they can cause because of their own inflated sense of self-worth.