Ask HN: How do you handle conflicts with your peers?
58 comments
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!
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!
I have managed senior engineers in almost every discipline who were SMEs in their area, and either much smarter or skilled than myself.
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.
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.
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.
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.
My issue with requiring a so-called "quibbler" to develop the proposed change themselves is it puts a high cost on making constructive recommendations.
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.
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.
> puts a high cost on making constructive recommendations.
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.
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.
I like this solution. It saves a whole bunch of rework. And it forces the person who wants the change (especially after a design review was approved)to put up.
How do you distinguish between "quibbles" and changes that are worth making yourself? I suspect it's quite a subjective thing.
I'd also suggest using a less disparaging term than quibblers.
I'd also suggest using a less disparaging term than quibblers.
Does it profit anyone to make that distinction? Unless the quibble is a
genuine nit (spelling error or one-line change), have the quibbler implement the desired
change. Oftimes the quibbler misjudges how much code would have to change to
accommodate his "minor quibble." Better to have him figure that out
first-hand, than the original author having to convince him herself.
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.
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.
> I just won an 8-year argument with my partner about what can and cannot go down the garbage disposal.
Do you mind sharing what you learned?
Do you mind sharing what you learned?
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.
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.
Completely agree.
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?"
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?"
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.
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.
> make the changes in the interest of overall productivity
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.
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.
> 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.
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?
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?
Consider this “refusal” an investment. There is technical debt and there might as well be social debt.
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.
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.
> Further, I don’t see how not doing unnecessary stuff as “fostering an unwelcome environment”.
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.
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.
[deleted]
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?
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?
The 'perfect scenario' I work on is to try working from inside to out. The first question is "Do I understand?" - maybe the goalposts aren't shifting and there is something going on that you haven't picked up.
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.
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.
My suggestion is to ask them to fully explain the reasoning behind their comments, rather than just lobbing change requests into your code submissions. You can frame it as wanting to learn, grow, and understand the processes necessary to create good code. They have to either shut you down and be unhelpful, or have contradictory explanations that they give forward.
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
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
Tips:
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.
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.
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.
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.
> Is he really just his own preferences or something widely accepted, but you are fed up with him?
> 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.
> 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.
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.
I ultimately quit.
In such situations I take solace in knowing the assembly generated by the compiler will at least have the same styling.
At the end of the day it's just text on the screen.
At the end of the day it's just text on the screen.
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.
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?
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?
This is the right answer. Thank you.
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?
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?
> How do you handle weighting benefits when the team is flat, i.e. there is no clear hierarchy?
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.
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.
> 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
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.
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.
I'm not talking about people mistaking juniors for seniors. I'm talking about when a reviewer lacks the context to understand what your code is trying to accomplish - which happens all the time, even on well-functioning teams. Those situations require you speak up and at least try to clue the person in.
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.
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.
> I'm talking about when a reviewer lacks the context to understand what your code is trying to accomplish - which happens all the time, even on well-functioning teams.
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!
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!
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.
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.
Recently we've come to the same conclusion with my team. With one small distinction, we prefix all comments with tags
[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.
[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've also adopted almost exactly this in all code review comments anywhere.
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 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..."
If the comments are non-blocking, I usually accept the diff and leave it to the submitter to do as they please with the review comments.
yeah I like this workflow. If I make comments, but approve, it means I'm happy to accept it as is, but you could address these simple changes before merging if you like.
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
- 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.
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.
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.
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."
* 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.
- 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.
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.
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.
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.
- (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.
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.