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?
Why should be bad PRs ever be allowed for any reason?
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_.
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.
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.
hell, that's a GREAT opportunity to get this "bad developer" under your umbrella and start mentoring him to be a better developer.
Teaching people cool new ways to code should be fun, not an exercise in showing everyone the size of your ego.
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.
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
Define bad PR. Are you sure that your definition of "bad" is accurate?