Remix.run Logo
Philip-J-Fry 25 days ago

I can tell you how that situation comes about.

You start by rejecting those PRs, saying "write more maintainable code, not quick hacks".

Management starts pressuring the original developer "why is it not merged yet, I thought you had it working".

That developer hits back with "well, it failed code review, they want me to refactor it".

Management goes back to the reviewer, "why did you fail this? It meets coding standards right? Pipeline is green".

Reviewer says "Well, yes it technically meets coding standards but it's full of hacks and is not future proof, it will bite us."

Management says "If we coded for tomorrow we'd never get anything done. Don't be so awkward". And then code gets merged.

Then you learn to just let these people go wild. If it hurts in the future you have a nice little "I told you so". But in my experience, management doesn't actually care if it hurts us in the future, it's not their problem. They just say "Well give me bigger estimates if you need to refactor". Fair enough, it's not a big deal but it is a pointless slog of picking up the pieces.

The other way it comes about is when the original developer just isn't really that good of a developer. So you end up in such an endless feedback loop trying to get the code in a good state that you piss everyone off and it's just easier to merge it.

Some hills just aren't worth dying on. And these guys can be exploited for your own advantage if you want to get code merged quickly ;)

alexandra_au 25 days ago | parent | next [-]

>You start by rejecting those PRs, saying "write more maintainable code, not quick hacks".

How do you go about that when for example, my previous employer just allowed any software developer to commit to any branch, and there was never any code review happening?

HeyLaughingBoy 25 days ago | parent [-]

Set rules on the repo. For example, no one on my project is allowed to check in code unless it's on a PR that's been reviewed and approved by at least two people. Attempting to push to /main will just fail otherwise.

pengaru 25 days ago | parent | prev [-]

wow, that's a charitable take

IME the manager just approves the PR themself to "bias for action", someone else will pick up the pieces