[lhorie]

Congrats to the author for realizing that confidence doesn't equate skill. This isn't being a pink unicorn, it's simply humility.

The real danger of confusing the two is that real skill dulls if you let overconfidence take over, since the very thought of being "better than others" prevents you from accepting that maybe there are things that you don't know or haven't considered.

For example, is "destroying" a coworker's commit in a code review really really helping? Did the person perhaps forget to consider that hiring to refill the position of a fired developer actually costs physical money (as opposed to time spent on coaching?) or that perhaps a lot of time is being spent on nitpicks without bottom-line value or that tip-toeing around egomaniacs can stunt autonomy and therefore creativity? This is not merely a matter of whether you're lulling yourself into some false sense of superiority/duty/whatever; thinking in terms of bigger and bigger pictures is an actual skill that one needs to develop when going up the career ladder.

[robohoe]

Humility goes a long way in life. Anecdotally, it's definitely something a lot of software developers and systems engineers should learn how to practice.

[blahyawnblah]

If you have to "destroy" every PR from a developer, you're wasting more time (money) than it would take to replace them.

Why should be bad PRs ever be allowed for any reason?

[n8cpdx]

There are many ways to solve any problem. If I reviewed every PR with the standard "is this exactly how I would have done it", I too would be destroying every PR I review. And I'm not even that good at coding - I'm the type of generalist who cares about things like the business beyond the code base.

What really matters is whether the code is solid and whether it will actually cause problems. Just because a PR isn't character-for-character how I'd write it doesn't mean it's bad. Based on the author's experience with the fired employee's replacement being just as bad, my guess is he was over defining goodness and raising the bar too high.

Bad PR to me means: bad runtime complexity, bad formatting, bad security, bad memory use, bad understandability. If it meets all those bars, which for a crud app is usually easy, then it should probably pass. I'll sometimes leave a "I would have done _x_ differently" comment on matters of taste _if I think it will help the other developer_.

[marktangotango]

> If I reviewed every PR with the standard "is this exactly how I would have done it",

This, too many people think their way is the only way. It only takes one cancelled project or one company going bankrupt (and 100,000s loc going "poof") to understand this work we do, really doesn't matter all that much in the end.

[JamesBarney]

I've spent over 6 years working on 5+ legacy projects that had millions of lines of code and maintenance teams 30 times smaller than the army of contractors who developed them.

And I've learned all the small nit-pick maintainability issues made no difference. Whether or not there was a style guidelines, and whether or not it was enforced made little difference.

Things that really made a big difference were giant bone-headed architectural decisions like "just save persist this data as serialized text", "lets write all the business logic in stored procedures", "building the software on a terrible platform".

And having some type of identifiable domain model helped a little too.

[BoxOfRain]

It is a really humbling thought that every single one of our entire life's work as developers is one unfortunate astronomical event away from vanishing as though it never existed. Kind of puts it all into perspective really.

[fernandotakai]

bad PRs shouldn't be allowed, but you don't have to be a smug a-hole while reviewing PRs.

hell, that's a GREAT opportunity to get this "bad developer" under your umbrella and start mentoring him to be a better developer.

[fayten]

Yup! I have been very fortunate to work with extremely kind and empathetic senior engineers in the past. They have done wonderful jobs at using the code review process to teach instead of tear anyone down. I am always shocked at how many programmers become so aggressive about development. It might be silly, but I think the xkcd comic Ten Thousand is a great mantra: https://xkcd.com/1053/

Teaching people cool new ways to code should be fun, not an exercise in showing everyone the size of your ego.

[rkk3]

Of course there are thresholds for competency. But if you find yourself having to destroy every PR from a developer you're probably toxic & insufferable.

[mjlawson]

When you're in these situations, you are faced with a challenge which isn't solved by code reviews no matter how illustrative, nice, or demeaning they are.

It's first solved by identifying what the cause is of their poor performance. Are they in over their head? Are they having personal problems which are distracting them? Are they experiencing burnout?

I've heard people say that they enjoy code reviews because it gives them latitude to be ruthless. After all, it's for the benefit of the business to not allow suboptimal code through, and it's just code. But frankly, I think that's a lame excuse and the easy way out.

There are people behind code, and while the code might suck, the people don't. Obviously, that doesn't mean you should let it through, but it does mean you should be at least professional, and preferably compassionate in how you review it.

[JamesBarney]

A bad PR can mean soo many different things.

Ranging from "I wouldn't have implemented it that way" to "this opens up a huge security hole".

Security issues shouldn't be allowed in. But a lot of "maintainability" issues have negligible if any gains, and real costs like

it takes time to fix

could introduce new bugs

creates bad feelings between coworkers

[nine_zeros]

> Why should be bad PRs ever be allowed for any reason?

Define bad PR. Are you sure that your definition of "bad" is accurate?