Question for OE devs - at one of my job...
Question for OE devs - at one of my jobs there are 2 developers who produce absolutely awful code. Like very bad. Almost every single PR they merge causes problems and will need a rewrite. How do I navigate this? I don’t want to be the asshole that makes them lose their job, but should I do it to cover my ass when they probably question why they have PRs in review so long?
6 Replies
correct-apricot•10mo ago
Like it’s gotten to a point where this guy currently has multiple PRs open for large stories, and I just don’t even want them merged. It’ll just cause more headache down the road when I eventually touch those files. Or am I just being an asshole?
judicial-coral•10mo ago
You gotta just notate the nit picks. Keep commenting on what it should be in your view. Having tests helps them prove their case if they truly think it's valid and if none, then you have the right to request changes
plain-purple•10mo ago
yep agree with TD. Coding is inherently opinionated, so it really comes down to test validation + whatever conventions your org wants to enforce. As long as it checks those two boxes, that's a "LGTM" as far as I'm concerned. If it doesn't check those boxes, focus on one or both of those areas in your comments
correct-apricot•10mo ago
Both good points for general SWE, thanks guys
I’m at the point of just now hitting mid lvl so still navigating everything
plain-purple•10mo ago
yeah just try not to make it feel "personal" to the people you're code reviewing. devs' opinions are strong, so if your concern is stuff like "this code looks ugly", and there's no enforced conventions in your org, pushing back on that is probably not going to get you anywhere
if you are in that situation, where code convention isn't a thing being enforced in your company, you could make a case to your leads/director/etc as to why enforcing convention would be beneficial to the team, and now you've got something to use in your code reviews against these people
if its a test coverage situation, you can identify the lack of sufficient test coverage causing these defects, and that way it isn't being explicitly targeted towards these individual devs. rather, it now becomes an issue of how to level up the team
extended-salmon•10mo ago
Geese is giving good advice.
In addition, try to explain why their code will be problematic. Frame it in the context of the code/problem instead of a personal thing. For example if they have bad variable names you can explain how misleading or vague names can lead to confusion. Further, if you have a style guide, refer to that. If not consider creating one to standardize things across your team.
This is not OE-related, just general SWE tings