Ask HN: How do you handle conflicts with your peers?
The question is a generic version of the problem I am facing in my team.
A senior person keeps leaving comments to my diffs which are at best can be summarized as his preference for doing things.
More than not, I end up making those changes, because as they don't bring any benefits or harm over one another; trying to convince the other person is a more effortful job and also alters the relationship with that person for bad.
I am looking for ways how you handle similar situations and how I can make best out of it.
58 comments
[ 3.5 ms ] story [ 116 ms ] threadBring that issue up with your direct supervisor on 1 on 1s, if you have them.
Is that just happening with you?
Is he really just his own preferences, or something widely accepted, but you are fed up with him?
I'd ask myself multiple questions and try to understand why does he do that, and also try to show some empathy and try to onboard more people on it. Work isn't only about coding, but also social relationships and navigating politics. You can learn a lot by facing the problem.
> Work isn't only about coding, but also social relationships and navigating politics. You can learn a lot by facing the problem.
I generally do think about these questions, I will still try to be more mindful about them.
In this case, it's probably just best to tolerate it. It's tempting to try to discourage that kind of behaviour but it rarely works.
First, there ought to be a firmwide linter and style-checker applied to all changesets. This avoids a whole class of reasons to quibble.
Second, and this is more controversial, outlaw any "this is how I would have done it" remarks, and institute a policy whereby quibblers make their proposed edits themselves.
I need to use git as an example since that's all I know. Quibblers fetch your PR [following this]( https://stackoverflow.com/questions/27567846/how-can-i-check...), make their changes, and push on another branch, and ask for your counter-review.
In the vein of "talk is cheap, show me the code," verbal nitting is too easy and hurts morale. Quibblers, if they feel strongly enough about their nits, need to make a proper effort to "correct" your code themselves.
I'd also suggest using a less disparaging term than quibblers.
This can be useful for dealing with the type of argumentative person who will find something wrong with anything. But for a busy person who's been assigned a PR to review, this methodology restricts them from offering feedback since they don't have time to develop fixes, only to suggest them.
Do you resolve this by ensuring PR reviewers are allocated enough time in their schedule to develop suggested changes?
There's another problem here. Requiring others to fix bad code instead of pushing back bad PRs to the original developer removes an incentive for them to write good code. If someone else will fix it, why bother? Do you resolve this at the performance review level?
There ought to be some middle ground here, where you can shut down actual quibblers while allowing legitimate feedback to be quickly given.
A "constructive recommendation" is fine, so long as the reviewer still clicks "Accept" on the PR instead of "Requests changes." Otherwise, reviewer must make the change himself. It's called "work" for a reason.
> Requiring others to fix bad code... removes an incentive write good code.
I don't think you understand programmers then. It's an exacting profession -- it's against our nature to embarrass ourselves like that.
The problem is that this makes you look like you don't know what you're doing even if you DO know what you're doing, and it prevents either of you from arriving at a better solution that might have been revealed with debate and discussion.
Unfortunately, on some teams, a strict hierarchy is imposed and you ARE expected to comply with a peer's instruction in all cases, and they might be closed-minded and unwilling to listen. In this case, just shutting up and making the change is the only option. It's very easy to find this out by just asking your manager if you are allowed to push back or not.
If this is the case, one thing that helps is to bring them in to give feedback early on - not waiting until you submit your pull request.
But if this is NOT the case, then you should reconsider your approach.
> they don't bring any benefits or harm over one another
That sometimes is the case, but sometimes not. Having cleaner, more consistent, more conventional, or more performant code are all benefits (obviously). But having to spend time rewriting tested & working code is, in fact, a harm. This harm may be outweighed by benefits of a particular change, but it is non-zero. This is something any senior developer on a mission critical project should know.
> trying to convince the other person is a more effortful job
Why do you think it's your job to convince them you're right? Does it have to involve anything more than stating your case, simply and politely? If you still can't agree, and the issue is too important to let go, why not ask your supervisor or ask for a tie breaker from a third dev?
> and also alters the relationship with that person for bad.
This is only true if the discussion devolves into personal attacks, like insulting and name-calling. As long as you avoid doing that, pushing back will probably increase the person's respect for you, even if they don't like it. Respect is orthogonal to congeniality; a person might respect you without liking you, and like you without respecting you. Which kind of relationship would you rather have on your team?
I haven't had this experience at all. In my experience, everyone on a well-functioning team has a good handle on what the skill level of everyone else on the team, and things like this don't alter that.
It's a different kettle of fish if the team isn't well-functioning, of course.
And yes - if you automatically fire off any change requested of you, then it does give the impression that you don't know what you're doing or don't care. I'm talking from experience.
Yes, this is why my approach is to discuss it with the reviewer.
> I'm talking from experience.
We have very different experiences, then. Fair enough!
But I still have a question :
> Unfortunately, on some teams, a strict hierarchy is imposed and you ARE expected to comply with a peer's instruction in all cases.
Or maybe fortunately? How do you handle weighting benefits when the team is flat, i.e. there is no clear hierarchy?
Say for example, two senior developers disagree on how to structure a piece of code, because each one finds it more readable his way. Now structuring that code would have implications on other dependent pieces, the team using it, and future development.
How do tou handle such disagreement when each member has a strong opinion about the issue?
First, is there no hierarchy at all or a hidden one? That's the first thing to find out - you need to know the rules of the game before you can play.
> How do you handle such disagreement when each member has a strong opinion about the issue?
If you're a dev, I've had luck in such cases asking for a third dev or a supervisor to step in as a tiebreaker.
Or you can find out who is the most knowledgeable and experienced person on the team in this one problem domain, and go with their opinion.
Disagreements are not necessarily a negative thing, but an inevitability that can become positive or negative depending on how you handle them.
If you're a manager, then you set the rules of the game. Just let everyone know what those rules are and make sure they are followed.
If, however, I have a sound technical reason to disagree, then I discuss it with them. 90% of the time, a solution can be arrived at quickly, with one of us seeing that the other's approach is the best one, or both of us finding a third approach.
For the other 10% of the time, I've found that both of us are making really solid points. So I rope the rest of the team in on the issue, summarize as accurately and fairly as possible what the dispute is, and what the position of each of us is (if I can't correctly state the other's argument, that means that I don't really understand it -- and that has to be corrected first). So far, every time I've done this it has led to a rapid resolution, often because there's something that both of us missed, but someone else in the team noticed.
I thoroughly disagree.
If the changes are indeed inconsequential and a matter if taste, it is a waste of his time to mention these things and a waste of your times to change them.
In general, I very much dislike the “oh well, it’s not that much effort for me anyway” argument. It’s poison and lets wrong people reign supreme.
I see no reason to give in to such behaviour.
How about the time you spend debating about your refusal to accept suggestions? Is it not wasted effort as well?
And what's the impact on productivity and team velocity of fostering an unwelcoming environment where petty team members are irrationally inflexible and refuse to take suggestions even regarding innocuous changes?
Further, I don’t see how not doing unnecessary stuff as “fostering an unwelcome environment”.
It seems to me as this topic is not so much about code reviews, but about how many people are unable to confront others. All for the sake of a “welcoming environment”. In reality everybody is upset, grudges are building, one schemes in the shadows, and everybody is upset anyway. If that isn’t “unwelcoming” I don’t know what is.
You're the one classifying other people's work as unnecessary fluff. If your colleagues went through the trouble to provide you with patches and suggestions them they don't see it as fluff.
And labelling being an uncooperative and unhelpful colleague as "social debt " doesn't fix the problems you're creating for the team.
We encourage people to mark their comments on diffs as to whether it should be regarded as a blocker to being merged, or not a blocker. (Perhaps poster could ask the Senior to do this)
Blockers should be comments to correct:
- code which goes against either the spirit of the spec or the spec itself, in other words, wrong behaviour.
- something which could really harm performance in a meaningful way (back this up with benchmarks)
- code which could break something else (probably a sign of a missing test elsewhere)
Non blockers should be the subjective things that are the topic of OPs post, but it's the responsibility of the reviewer to make that clear.
The committer can then decide whether to implement or not.
We couple this with quarterly patterns and practices meetings where we can discuss any of the subjective things which come up over and over again as points of friction. If we can come to an agreement, great, if we can come up with a linter rule to enforce the decision automatically, even better. Then it's removed as a point of interpersonal friction.
There's a culture I push for in the company I work for and in the teams within.
It's to try and best as possible, to check your ego at the door and regard the code as 'the companies code' rather than 'your code'.
This is important to strive for in your Seniors just as much as less-senior developers. (Rank shouldn't be blindly used as barometer of the decision whether to hear out feedback or not).
When you keep focus on the culture, working practices start to emerge rather than trying to dictate them from the top down.
[nit] : i would do this differently (opinion based) (for vs foeach for example )
[tip] : for future reference you can do this in x or y way, or you can use foo framework/language feature, but your way is OK to
[blocker] : possible bug,performance,security business logic issues
[Q(uestion)]: what is the reasoning behind this code ( could also indicate need for documentation)
the nit & tip ones we don't force anyone to fix or even address, but all questions and blockers must be answered & resolved.
I try to make my intentions of the comment explicit, saying things like "this is a minor nitpick, but..." or "as a tip for future changes, this can be done like..."
I recommend a book called "Crucial Conversations". You can find it on Amazon. It's a fantastic book about how to recognize that you're making "The Fool's Choice" (their words)... and how to get out of it.
To apply the book to your case, you've boxed yourself into two options (I.e., The Fool's Choice): 1) I can argue with this person and could have an unfavorable outcome, or 2) give in and just do what they want. This is actually a false dichotomy -- there are other options! One of the possible options is to turn that choice into a question by turning them into an AND option instead of OR options. In other words, instead of asking yourself "Should I make the code change OR make them hate me?" you should ask: "How can I nudge them toward my way of thinking AND _not piss them off_?" This alters your frame of mind, and you would approach the conversation much differently than you would before.
Anyway, that's the gist of it. The book is fantastic, though, and I recommend you read it. It's a pretty easy read.
Good luck!
A way I dealt with this was to ensure the discussion was on the project/problem - and that people had input on the solution regarding its feasibility.
i.e. "OK, team, tell me why or how this solution can fail" and "OK, what is the negative impact of doing it this way?" "Can you see any holes in this method?"
The idea is that people can voice concern about failure conditions for a solution - and it keeps them focused on the project/problem/solution and less on their ego or interpersonal communication between team members.
So if an engineer wants to do something that will have an impact on someone else should that thing break, then it can be weighed into the value of that solution.
* Sit together as a team, agree on 'best practises'. Summarize them in a written form and refer back to it. Avoids having the same discussions multiple times.
* Try to automate all that can be : For example using an auto formatter avoids the 'this method is too long, or the curly braces come here discussions'.
* If it is a specific problem with someone, talk about hoew you feel about the behaviour, not whether it's right or wrong. Explaining how you feel can help the person in front understand the facts without having to specifically agree. Feelings are irrational by nature.
In the end though : "Arguing with a DEVELOPER is like wrestling with a pig in the mud. After a few minutes you realize the pig likes it."
- Many stylistic choices can be handled with linters, formatters, styleguides and the like.
- Some append a nit: (nitpick) to the comment: “Do with these changes as you please”.
- Picking on trivial stuff is akin to bike shedding. It takes little to form an opinion on the trivial while the difficult parts are often ignored.
- Simply point out that this is a non consequential thing.
- Define a CR manual for your team/group/company/whatever. Talk about what should be done, what ought to be ignored. An open discussion often sheds light on people’s motivations.
- Pair with said dev and talk it through in person.
- He might just be a bastard, in which case normal solutions just don’t work anymore.
- Agree on a team policy for code reviews, formatting, style, etc. (this alone should eliminate most of the subjectivity)
- Read and share this https://google.github.io/eng-practices/review/ with the team
- Design discussions could be agreed on before code is written
As for conflict in general... communication. Be absolutely direct, but not confrontational. Do not be passive aggressive, or round a bout in your words. That's always worked for me, at least - not for getting my way, but getting clarification in the shortest time possible.
What I'd do there would be make the change, then shoot off a quick message along the lines of:
"Hey, I've made that change you suggested. A question though, I'm not sure I completely understand why you prefer it that way? Is it just a code style/consistency thing? Or is there something deeper I don't understand? If it's just style, do we have a style guide I should study? If not, would it be worth while me starting one as I learn these things along the way? Might help future new hires get up to speed quicker. I'd be happy to take that on if you think it's be useful?"
- (Onboarding)
-- Making sure you use common style / naming conventions so that code is readable as different developers work on it
-- Making sure you use common design patterns / dependencies / collections / ect, so that as different developers come in and out of a module, no one needs to re-learn things that just need to be the same
[edit] -- Sometimes it's just hard for newcomers to know all the gotchas and longterm consequences. These should be patiently explained.
- Readability: Make sure that variable / class / method names make sense
- Ask questions: Sometimes a question in a code review means you need to add a comment in code
- Knowledge transfer: A reviewer may have more experience with a specific API / pattern / technique
Without knowing the specifics of your situation: It could be that your senior colleague just can't "let go" of the fact that they didn't write the code; or it could be a situation where your changes just don't fit with the overall architecture, style, ect. Most likely, one or both of you need to swallow your ego and prioritize the code over your own personal preferences.
Side note: In general, you should always follow existing style / patterns / conventions in existing code, unless there is a very clear tangible problem with it. (IE, if all variables have 1-letter names and there's no tabbing, you probably don't want to follow style.)
Anecdote:
A few years ago, I got a surprise review request from someone from another team. They completely bypassed our dependency injection pattern and did "their own thing." The way they did it was a perfectly fine pattern, but it didn't match the pattern with the rest of the product. Long-term, it would create a huge maintenance problem if there were 1-off modules that used different patterns.
I'm sure the other party thought that my review "can be summarized as his preference for doing things," but that wasn't the case: I was more concerned about long-term maintainability when other people needed to maintain their new code.
Anecdote 2:
In other cases, I sometimes block a review on confusing variable names. Yes, naming a variable "file" might make sense to a newcomer, but after working with the codebase for a long time, I know that "fileHandle", "filePath", ect, are much more readable.
[Edit 2]:
> also alters the relationship with that person for bad.
Talk to your manager about that.
Force, withdraw, smooth, compromise, and collaborate.
I don't like this model, I prefer the power, mediation, rules and needs model because it is more action oriented especially for people who aren't managers. If you think there should be one of them that doesn't exist (for example a rule) propose it up the chain, then use said rule to resolve the conflict. Sometimes maneuvers like this take time, for example if you want to use power but don't have it, it can take time to get enough to use it the way you want (and you need to think hard about if it is worth the potential fallout).
Generally use empathy and discussion to use needs first, rules second, mediation next, and power as a last resort. That's my approach at least.
But really talk more in your team. Otherwise it is a disfunctional team. Agreeing on a ruleset for a linter is a start.
Other idea: draft a „code review manifesto“ with all team members and let everybody commit/follow this manifesto. Again it will be the consensus of the Team given by the manifesto/linter and not the individual person in case an argument comes up.
In other words, incessant, aggressive, and unsolicited, with the reviewer not backing down even after you've implemented their changes? (e.g. Their previously implimented suggestion now are the wrong approach according to their next round of comments.)
I've tried simply going along with it, recognizing my own technical inexperience compared to theirs, but end up getting nothing done because of the capriciously-set goalposts.
This has been a recurring problem for me. Any tips?
Checking that you understand involves finding evidence; so now you should have some evidence to go to someone else and ask "Do I understand?". They can help review the evidence and provide a check that it is convincing or at least realistic.
Having been thorough and made sure that there isn't something to fix up on the easy side, the next step is going to the reviewer and (gently, slowly but directly) raising concerns they are giving unhelpful feedback. An easy trap here is having evidence while they are going in blind which is a rather threatening imbalance, so give them time (maybe a few days) to think before expecting a response or there might be fireworks.
That exhausts the social work, so if the situation is still unresolved consider if it is reasonable to formally escalate the evidence to someone somewhere in the command chain so that they are aware the reviewer needs some coaching.
That covers everything that can be done short of quitting.
Hopefully it actually is a good-faith interaction and just a breakdown in communication. If not, though, putting someone on the spot to explain their actions is a good way to generate documentation that they're acting in bad faith.
This works with mean jokes, too, fwiw
Relax. Detach. It’s just code. You just finished it and you want to be done and be victorious and move to the next thing. That’s understandable. But this is a long game.
Never reject advice immediately. “So, I’m not sure I like this, yet. I’m probably missing something. Let me think it over and play with it tonight, and let’s discuss this again tomorrow.”
Assume good faith. Try to give them the benefit of the doubt. Can you explain back to them why they want the changes, in a way that they agree with? Try it. Even if you disagree, they will see that you have heard them and understand their perspective.
If you disagree with their perspective, you can ask them about it, and why they value this over that. Keep an open mind. Later, a good question is, “Do you think we need to worry about <counterpoint>?
If you are sure they are wrong, consider using priorities or authority to deflect. “I could make these changes. Do you think that I should do them before or after ___?” Make sure you know exactly what goes in the blank, i.e., what are your organization’s priorities anyway? Or, “I think I will need to talk to my manager before I spend more time on this, he really wants me to focus on ___.”
If it is dysfunctional, get help. Tell your manager what you think is happening and ask them for advice. Can they see a way to make this less stressful? If it is really bothering you, your manager should care a lot and should help you work resolve this. If they don’t, you have a crap manager and should make a change.
Does this imply the other person thinks their suggestions bring significant benefits? (If not, they are already aware of the problem and may just need an explicit reminder.)
If they do (and you are sure they are wrong about it), it sounds like they have a miscalibrated sense of what matters -- this would show up in all reviews they do, i.e. affect more people than you. If it doesn't, it could mean more people have this miscalibrated sense.
What I'm saying is that you might have just unearthed a huge systemic flaw in your teams review process. The worst thing you can do now is ignore it. Make a thorough effort at creating agreement within the entire team about what the goal if your peer reviews are, and which types of things are important.
Without examples, it’s hard to say whether this person is being petty or believes that in case of a production issue, they will be the one who is called upon to fix the problems. Possibly both. Cleaning up messes isn’t fun, so people can sprout a lot of opinions.
If you were a coworker I’d have you send me reviews to look at, but you’re not in that sort of scenario here. Is there anyone who can talk to him? I got pushback early on about my review style and one thing I figured out is people don’t want to hear things from the same person over and over, or hear about everything all at once. I learned to let my coworkers make all their comments, and come in late and point out only the things they all missed. Smoothed a lot of feathers, and honestly saved me time and energy (and a bit of stress). Your coworker might need to hear this from someone other than you.
I just won an 8-year argument with my partner about what can and cannot go down the garbage disposal. Cost me a couple hundred bucks for an after hours plumber, but now she knows. She thought I was just being fastidious even after I told how I learned these rules. Sometimes work is also like this.
Do you mind sharing what you learned?
If so then maybe defer his changes until they are implemented in linter and/or CI job configuration preferably at a company, team or project level.
If this is something more debatable like how a feature was implemented (there are many ways to skin a cat, all of them wrong). Then I would suggest they provide their suggested changes and maybe even some BDD-style tests that would not pass given your implementation.
If they don't have the time and still insist on you doing things 'their' way, maybe inform your product owner or project manager that you are spending extra time working on this feature but it will not result in delivering any additional value to your users/customers and have the reviewer take up the discussion with them.
to me is so much better than
"A senior person is approving my PRs and I'm not sure they've even read the code"
'preferences' should be addressed by a style guide.
You should be able to work to an agreed team style, even if it is not your preference.