How do you handle conflicts with your peers?

This isn't specific to code comments, but one soft skill that everyone should learn is how to have conversations with high stakes. The code changes might not be high stakes, but having a good relationship with your peer is very high stakes.

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!

Nature of the beast, I'm afraid. The "beast" is group programming.

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.

Fire safety folks are very opinionated about how buildings are laid out. It may seem arbitrary to you or I, but you’re not the one having to pull people out of burning buildings.

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.

So, I don't feel the senior person in this case is necessarily wrong. So many languages give you 30 ways to do anything, it's key for a team to agree on standards so that code is more easily read by everyone on the team. If you disagree about the style being suggested, don't argue back directly, but instead try to set up a quick chat with him/her and/or the team to suggest a different way of doing things. Having each member of a team having their own unique style gets annoying and confusing, to be honest.

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.

If a team member is requesting changes that don't cause any harm, then I tend to just make the changes in the interest of overall productivity.

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.

So, I have a related question: What do you do when the comments aren't being made in good faith?

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?

Do more pair programming(preferably with him) and avoid having that issue at code review.

Bring 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.

I was at a company that had no style guide or coding standards to speak of. If I raised the issue that we really should have a style guide I would be told, "I don't like other people telling me how to code." Code reviews boiled down to, "just do it my way because I said so, trust me."

I ultimately quit.

First thing that comes to mind is can any of his suggestions be enforced with linting and/or CI jobs?

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.

I used to handle this just like you (just shutting up and making the change). There's no specific technique I can give you other than to question your own assumptions.

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?

Where I work:

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.

Some quickfire help:

- 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

> 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

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.

2 tips for code reviews:

1. It's all a give and take. Implementing changes your peers request makes them understand you value their opinion. This can make other interactions easier - e.g. the ones where your standpoint is important to you and you don't want to derive from your opinion/code/design/etc. Save your energy for those discussions, and lean in on proposals for which your passion is lower.

2. If you do not get a common opinion after 2 review comments back of forth go to the person and talk them in person. If you or they are remote, give the a call ("Hey, do you have some time? I would like to know more about your suggestions"). Then everyone can present their standpoint again. Sometimes that already helps, because things get lost in comments. If there is still a disagreement it's still often easier (and definitely faster!) to work out.

I would go for a walk over coffee or tea and try to understand the reason for those comments. Walk and coffee makes it informal which shows non conformational approach. Something about walking makes it an easy conversation compare to sitting in the room face to face with looking at each others expressions.

It really depends on the situation, but in project management the list of conflict resolution goes like this (not in order):

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.

A few ideas :

* 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."

A few things:

- 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.

Try to talk together and introduce a tool like a linter. Then let every member commit to the linting rules which should be used by the linter. This way you can decouple the review process and it will be the linter‘s fault and not a fault of any engineer.

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.

If they're largely stylistic changes like whitespace, loop style, or something of that nature, you could just ask them to write up a doc on what they want. Many companies have such a document.

If I was your manager I’d like to know about stuff like this so I could try to help figure something out. Unfortunately it would feel like snitching for you and your actual manager + coworker might feel the same way. I usually try to avoid being a “problem employee”, so often end up just going down the path of least resistance. It’s a tough situation, especially if teammates who are senior to you struggle with leadership and decision making.

> A senior person keeps leaving comments to my diffs which are at best can be summarized as his preference for doing things

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.

If you picked that person to be an reviewer, that means you value their preference, and should at least make a faithful attempt to understand about that preference. If you haven't picked that person as a reviewer, then the merge it in.

Robert Greene writes books on this, on how to tackle conflicts. 48 Laws of Power and 33 Strategies of War specifically.

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.

Why do you assume that a) it's just about the senior dev's preferences, and b) they must be arbitrary, and you actually know better?

You talk to that person.

Sounds like you are actually handling it well.

Be excellent to one another, dudes.

A code review / pull request is supposed to be a discussion among professionals about how to make the code the best it can be. This includes:

- (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.