Several times in that span, I've worked with people who push back quite forcefully on comments or feedback on topics or practices that have either explicit company standards or at least generally accepted best practices.
It seems to me that most people at the company are conflict-avoidant so that when these people push back aggressively, it's often a successful strategy. Ten minutes of arguing and wearing the other person down saves them 20 minutes of writing unit tests or fixing their code or whatever. I've seen in the past that these people effectively carve out an unspoken exemption for themselves because everyone is sick of having the same conversation.
I'm the tech lead but not manager of my current team and I'm responsible for the technical execution of the team and the success or failure on that level. There are a bunch of great people who do great work and are pleasant to work with but there's one person who is so unpleasant to work with. I hate that I have to constantly remind this person of company/team/professional standards and that it feels like every conversation is an argument (examples below). I hate that this is in my head on the weekend.
Questions:
- I try as much as possible to explain _why_ practices are what they are, the effects on our project, the team, etc. Any suggestions for these conversations in the future?
- I can't control this person's behavior but I can control my response. Suggestions for dealing with this personally so I'm not wasting my Sunday thinking/writing about it?
- Other thoughts or feedback?
Thank you so much in advance!
Some examples: - Sending a large PR that changes many files at once because their changes kept growing in scope as they were trying to figure out how to do something. Company has lots of guidance about small changes being easier to review, less bug-prone, etc and how to break them up. I try to emphasize the benefits for the team and codebase, the respect for the reviewer, etc. Generally get push-back like "What does it matter?", "It's already done.", "It would take too much time to break up.", "I'll do that next time", etc.
- Adding unit tests for some piece of logic. The benefits of unit tests are so fundamental, but I try to emphasize that there are many people working on the codebase, don't want to accidentally introduce bugs, protect that logic for the future, etc. Generally get push-back like Well it's so simple. It's not worth testing. I'll add a test later. etc
- On Friday, I discovered a chunk of code copied from Stackoverflow. It was crappy code, which is what caught my eye in the first place. Company has clear guidance that if you want to use outside code, we must verify the license and segregate it from owned code. (If you're curious why: it's hard to know where the SO code originally comes from, maybe copied from a closed-source project or one with GPL license or whatever, and even if it's original to SO, there is a license for that and it wouldn't be considered owned by the company). I was shocked to receive push-back on this. The person said things like "how would anyone find out", "what does it matter", "everyone does it", "it's so low risk, who cares"
Empathy is super effective in this situation - use the magic words 'it seems___' and get their reaction. 'It seems like you're trying to get as much code done as possible and see these policies as busy work'. 'it seems like you don't think the quality of the team's output is how you'll be measured'. Sometimes there is legitimate misunderstandings (were they told they need to write x lines this year to be promoted?), other times they're missing an implication and need a reality check.
> I'm the tech lead but not manager
You need to have a conversation with the manager. Document the violations of standards and company policy. The manager should review this with the problem employee, in writing, and the employee should sign an acknowledgement that the warning was received and understood.
If the employee does not improve he or she should be terminated. Such people are like cancers if they are allowed to remain unchanged.
Ideally the "large, famous" software company already has defined an HR process for managing situations like this.
If you can't get backing from the manager or the manager's manager then you need to think about whether you want to stay in a company that isn't giving you the support you need to be "responsible for the technical execution of the team and the success or failure on that level."
To me, this reads as a toxic person who would try to get away with these things if they hadn't been caught, and who could lead to a massive shift in ethics and morale if allowed to proceed unchecked. That's really alarming. Imagine if a person with this attitude was in a position where they were responsible for code safeguarding PII or other sensitive data (and arguably, this may already be true, e.g. if they were to introduce a security hole in their component). It's essential that you document everything contemporaneously (or as close to that as possible), ensure the person's manager first knows your concerns in private - and if they don't take any action, give them the heads up that you'll need to escalate to your manager about potential ongoing legal risks. It will be annoying to navigate the politics, but it will be much more annoying if the wound festers.
Frankly, there's a lot of talent out there right now with great ethical compasses and experience with large codebases, and allocating headcount to a toxic person simply isn't optimal in that context. And even if you're in a situation where a dismissed team member won't be replaced e.g. due to hiring freezes... this person may very well be having a negative effect on the team. Just first make sure that they're not someone's kid... and good luck!
1. There are code bases and PRs where coalescing many small changes into one "this changes how we do this" commit is encouraged when it's a semvar level change requiring coordinated edits to keep working, rather than a purely iterative change.
2. Most research shows universal unit test coverage is lower ROI than judicious coverage of intefaces and risks.
If many of your discussions fall in this zone, it's possible you, yourself, may be taking guidelines as too black and white.
Meanwhile ...
3. That one is just bad.
1. Why are they pushing back on it? Have you asked them what they consider up to standard? If they don't have high enough code standards and think it's OK to push crap, then there is a clear expectation gap that you should identify to them and tell them they are not meeting the bar.
2. Did they have any say in these standards? Sometimes you can turn an enemy into an asset simply by having them own a piece of the standard (and holding them accountable if that standard doesn't hold up)
3. What do they suggest in lieu? Maybe it's a stylistic thing, maybe it's a quality thing. Maybe they don't understand the difference, or maybe their impostor syndrome is making them defensive.
My summary is that you need to ask probing questions to fully understand their perspective. THis is difficult because they sound defensive, which is reasonable - your approach sounds a little like an attack due to your frustration. I can assure you they are frustrated as well!
Finally, keep in mind to set up the right context. When the light is red, stop pushing. Find a way to have an open and constructive dialog - you will have to bear that responsibility, but once you can find that, you will probably have better luck.
And document each of these steps, in email, to the person. Then when you go to leadership, you can share what you've done. If they ultimately just don't meet the bar or expectations, they should be let go.
If you're not willing or able to clash, you'll have to try find some compromises that improve the situation but make for an undesirable choice for this person. eg. In the PR example, require that large PRs require an in-person walk through.
Try being more blunt & concise. Instead of explaining, just say “please add unit tests and I’ll approve,” and then disengage. They’re the one who needs their PR approved - not your problem.
The key here is escalation path. He’ll have to bring it up to his manager, and then the conversation is with his manager, and then you can have a broader “he can’t just ignore every standard” conversation.
With that said, code quality, F/OSS license policies, and testing should be something the whole team talks about and comes to an agreement on. There's a big difference between pushing back and failing to meet the team's basic standards, and at a certain point it becomes a performance issue.
It sounds like you already made up your mind about this person and came here to enforce your confirmation bias.
Since I've been on both sides of this story, I'm going to be the devil's advocate.
> I try as much as possible to explain _why_ practices are what they are, the effects on our project, the team, etc. Any suggestions for these conversations in the future?
How about showing the hard evidence that those practices are beneficial and a net positive? Let me guess, you don't have that. If you had, there wouldn't be a debate. My bet is you don't even have metrics to look at in the future that will prove your practices are beneficial.
> Sending a large PR that changes many files at once because their changes kept growing in scope as they were trying to figure out how to do something
Is this a one-off you're looking to attack them on, or a consistent behavior? Based on the description I suspect it's a one off. If it's consistent, it is pretty bad. Since it looks like you're already got off the wrong foot, you'll need other team members to advocate for smaller PR.
> The benefits of unit tests are so fundamental
Ah, the cargo-culting. The benefits of testing are fundamental. But testing is hard. Unit testing is a way for lousy engineers to pretend to do testing.
https://kentcdodds.com/blog/write-tests
> On Friday, I discovered a chunk of code copied from Stackoverflow ... The person said things like "how would anyone find out"
Was it a 2-line or 100-line snippet? Context matters.
The person you're describing might indeed be a toxic asshole you want them to be. But it is also quite possible that you are the asshole here, mischaracterizing them to confirm your bias.
These mainly seem like lame excuses that are being used to shut the discussion down. You don't need to let that succeed, you can potentially engage politely but firmly.
"What does it matter?" => It matters because its easier to review, revert, reason about in the history, it's our convention that was discussed/communicated earlier...
"It's already done." => No, it's not, it is a PR pending review and feedback. This is my feedback. If you would like feedback from the group earlier in your development process, feel free to approach any one of us for input as you are making your changes, there is no need to wait until you feel that your change is "done".
"It would take too much time to break up." => How much time do you feel it would take to break this up? Are you familiar with the git operations to perform this task? Would you like me to help, or break it up for you? Realistically, it should probably only take a few minutes to perform this task for someone proficient with their tools.
"I'll do that next time" => Since you understand the rationale and benefit, why wait until next time, let's do it this time?
"Well it's so simple. It's not worth testing." => Great, since it is so simple, it should only take a few minutes to write the test.
"I'll add a test later" => Ok, when in particular were you thinking you would like to do this? Add a ticket for that, with a due date, so it doesn't get forgotten? Perhaps it's easier to just do it now, what do you think?
I'm the most senior developer in my team. Normally I block the PR with a change request and say exactly what I want, i.e "Please commit X and Y, in a new PR" and I give exactly the commands that this developer must run. Then, after this small PR is merged, I ask the developer to rebase the huge PR with the master, and do that until the initial PR get as small as possible. If you are not the manager or not team lead, the easiest way is to lead by example, mostly by doing together. Sometimes it can be frustrating, but I'm sure I bring those stuff up when I'm negotiating my salary raises...
The key is to
1) not engage in conversation
2) add mechanical tools that fail the build if coverage isn’t enough!
A lot of times people argue that it’s stupid to have a “target” code coverage. I used to be one such person until I saw what happens when you leave it to people. Most people would still keep quality bar high, but one rotten fish spoils the pond and the tools that mechanically measure quality metrics are very effective in pushing back on bullshit like this.
Oh and whatever you do, don’t engage in debate on this. They /will/ wear you down.
I’ve been here.
Resorting to authoritative measures (giving commands) is the only way. If you don’t need to pull the manager in, it works better. When they simply refuse to do the task you’ve agreed to and just do something else, you go in and solve their task, and you raise a concern with the manager. But to the extent that you can have the dialogue directly and make them sound spoiled will carry more towards equipping your own authority.
Ultimately, they are measured by their ability to solve problems they’re asked to in the ways requested.
The rest of it, I’d follow Galxeagle’s advice. Lay out the standards, lay out the consequences, have empathy, do not argue with them. Humane, but firm.
It seems something makes this person tick but it hasn't been figured out what that is.
It's not about the contents of those disagreements. It's about them not being in a collaborative mode. It might be lack of creativity, lack of autonomy, lack of perceived excellence.
Just my two cents with the little info I got here.
1. review the PRs and don't accept them if your requested changes aren't done
2. report to the manager
If they start getting their performance reviews taken down a rung or two explicitly due to not doing what was expected of them and communicated to them (repeatedly) then they might get the message. "You didn't get a pay rise this year because you didn't do what was asked if you on
In many companies, policies are guidelines for management to cover their ass. E.g. "Giving gifts to political leaders in third world countries is prohibited and illegal". This is complete nonsense, every MNC above a certain size have no choice but to grease the wheels when operating in developing countries. That's just how it is. The policy is there so the lowest ranking middle manager in the chain of command can get thrown in front of the bus if for whatever reason there's regulatory scrutiny. Same thing here your situation, if you gain a reputation for being difficult, then if the projects fails for whatever reason unrelated to engineering, your colleagues would be happy to point their fingers at you. Don't be that engineer with poor social skills. No business manager appreciates a stickler for rules outside of legal and compliance departments.
Short of that, manage up - tell the team manager that this person is a drag on the team by not following standards, demanding special treatment, being generally disagreeable. it hurts team morale even if the junior team members don't say so (they may not even realize it themselves, but it is a real thing).
In the shortest term, as others have said, don't engage. Pick the rules you enforce, and enforce them. Request the additional changes on the PR to meet style guidelines and don't take no for an answer. Don't get into discussions or arguments about the merits of the standards, just say "These are the standards and we're all expected to follow them." If the standards are written down (and if they're not, then they aren't really standards) then cut off any discussion on the merits of the standards with links to the written standards. Use a "chore" conventional-comment (https://conventionalcomments.org/) to indicate that it's just a thing that has to be done.
If you can, make it someone else's problem. Refer them to the standards committee or whatever senior architect is responsible for guiding the standards, and let them handle the argument.
Our CI does a check to see if Terraform was correctly formatted prior to push and will fail before any "init" / "plan" / "apply" step is reached.
Say something like the following. Just remember to include specific pieces of evidence that support your claim.
"Person X doesn't seem to care about company policies regarding the code quality. On following occasions: he/she did something sloppy that was against the policy A and refused to even acknowledge it during the review. Can you explain him/her why following these policies is important"
If you can adjust your behavior to compensate for what they perceive and you then want to, you can do something about it.
If you are the tech lead, don't you review all his changes? Just stop letting him add anything to the production code.
Give them a warning, document the constraints and say: "I am not able to add any of your contributions to the project going forward. We will be reviewing your inputs to the code on an ongoing basis. The quality of the work is unacceptable. I am not going to be arguing with you any further. Here are the standards. Anything that doesn't follow these is going to be rejected. IF you are unable to follow these, then I recommend transferring."
Then let him take a vacation and push no code and then slap him with a PIP.
One time, a manager asked me why I rejected a PR at the end of the sprint, before the demo, and I said, as you wrote above, that it's my responsibility and if he wants to overrule me, he should do that officially by assuming responsibility and escalating to our boss.
Make sure the rules are clear and then stop trying to appease.
edit: I wanted to make it clear that the rules were broken only when a new dev joined the team, after a rejection or two, PRs were extremely rare instantly rejected.
My advice: leave it alone. Focus on yourself and your code, improve your skills, and look for another role in another company. You'll be happier in the long run for the only reason that you can only change your behaviour. You cannot help others, I've got the scars and burns to prove it.
Does this mean its an actual standard? You can fail the PR for not following standards.
> Adding unit tests for some piece of logic
Are unit tests required? Fail the PR if it doesn't have the tests. If you decide to allow a "I'll do it later", only do so if they have a ticket, assigned to them, in the backlog.
> I discovered a chunk of code copied from Stackoverflow
Now it's a problem for HR, because they're exposing the company to legal risk.
All of these items are things that should be passed to the manager every time they happen.
Yet... no one ever - literally - pulled down code in a PR/branch and reviewed it locally, never reviewed any tests (in 2 years I never got one question about a test as in 'you missed use case X in your tests', etc). They literally just meant "reviewing this on the screen in the github UI is hard when there's more than a handful of files - it's hard to tell where the changes are or what they effect". This is why I would also have tests and (sometimes extensive) documentation in the PR, to help a review. Never happened.
I had "large" PRs (by someone else's labelling) about 10-15% of the time. I would get pushback - "this is too much - it's too confusing".
Me: "OK... let's schedule some time to review - maybe you can help me cut it back some, or you can assist me in some way in this work?".
Others: "don't have any time for that - this is just too much work".
OK, so... complain about the work, but when I ask for help to conform to your standards... I get a refusal.
I point blank asked multiple times when this was lodged as feedback - "what steps could I take to have made this smaller?"
The only substantive thing that would have made it somewhat easier to roll out some of these "larger" changes in smaller chunks would be to have had a more comprehensive feature-flag system in place.
I put a basic flag system in over a weekend - some server side, and some hacky method to respect the flags in client side code as well. It was done over about 3 hours, and was never improved. Again - very hacky. I had an "improvement" ticket in for... about a year, begging for more than a few hours to devote to tightening up "feature flag" system to be a bit more comprehensive, documented, etc.
Every "sprint planning meeting" it was the same "there's no time for that, it's not that important, quarterly deadlines, blah blah blah". But if/when I'd do things that were 'unticketed', that was met with accusations of being passive/aggressive, or being subversive, or 'not a team player'. "If you've got time to do that, you could have been helping out people who were behind". The same people who didn't write tests or documentation, and refused to meet with me when I asked for assistance in trying to accommodate their calls for "smaller PRs".
First year on the project was good, but as it grew, policy/procedure/scrum/agile got in the way, with more process and less ability to get useful stuff done. Second year was far less productive (at least, compared to what it could have been - I'm sure some people just saw forward progress as 'good', but we were greatly slowed by more process and ceremony over time).
If it's only you trying to block a merge, perhaps deputize others to co-review. Hearing your same objections from multiple others could make them harder to ignore.
Lastly, consider that maybe they are saying something that should be heard. Maybe they are right and it just appears to be foot dragging.
This one is easy to solve. Having automation or, as usual, blocking the PR with a change request and having it blocked until the necessary code get the tests needed. Again: Maybe the developer will add it, maybe you can do it in a pair programming session, or maybe you just submit the tests to the PR and lead by example. Sometimes it can be frustrating, but I'm sure to bring such proactive actions up when I'm negotiating my salary raise..
You build not only a strong list of counterpoints and points (without a single mention of the forcefully person's name) then elicit as many peer supports as you can with that list (preferably on paper that you can take back). Sometimes, I've had to remove the forcefully person's lone point in order not to tip my hand.
Then reveal in a meeting, again without mentioning the forcefully person's name.
I've gotten 100% this way, even the forcefully person's silent resignation and it's "aye" once the consensus is revealed.
Surprise is the name of the game.
If you cannot pull up a decent list and a consensus then you probably should just move on, unless it is a safety factor or a danger to one's life.
Route around them, which may include transferring or finding another job.
In the ideal world you would be their manager. Right now responsibility and accountability are separated, which is most likely causing you stress. Bring this up with your manager: tell them your plan to deal with the situation, ask for input and their backup. That way you now have a bigger mandate, and you don’t get in trouble for being tougher than you have been so far. Managing up is as important as managing down.
It might sound odd to have to lay down the law of the land, while simultaneously advising you hear them out. It isn’t. People generally want to be heard more than they want to be right. Show them you hear their concerns. Let them bring all their frustrations out. Tell them you’ll try to improve the situation for them. But also say that you cannot have somebody on the team who doesn’t abide by the policies set, and this behavior cannot continue. The content of the policies is irrelevant to that point, it’s a clear chain-of-command issue.