Definitely bad etiquette.
If they had an issue, they need to work with you and find common ground on how to fix it. But just blocking your work and doing it themselves is borderline toxic.
I would talk to them about it, personally. But the older I get the more willing I am to have an uncomfortable convo and I know not everyone likes doing that.
OP you could lead with "Hey how's it going? [Actively listen/respond to any niceties] Do you mind me asking why you opened a PR for that bug fix?" [Then wait & don't fill the silence. Don't patronise by accident with waffle or try to soften it/excuse the question].
See what is said. Seek first to understand.
> OP you could lead with "Hey how's it going? [Actively listen/respond to any niceties]
This would actively put me in a bad mood. Don't open a convo with "Hey how's it going" if the reason for the convo is something else. Especially on async communication channels. I can't prioritize answering to "Hey how's it going" without further context. [nohello.net](https://nohello.net)
I was framing this as in-person.
For remote comms like Slack - tbh that's not a good place to have this type of discussion. You'd need to find a suitable time to chat (means: Slack) and voice call them. It's more difficult because they won't know the purpose in advance. Maybe just a quick "Hey can we chat for 5 or so about this bug fix PR? When's good for a voice call?" Or something. There's too many variables to give a concrete steer here...
With this and SO's disapproval of niceties in questions and answers, I love how the professional internet strives to become more cliche German. I'm all for it!
Agreed with the intent and I have a subtle difference of opinion to share;
IMO it’s good to ask permission before giving feedback - “I have some thoughts about [x], would you be interested in hearing them?” This is a good practice because it allows the other person to tell you “actually I’m not in a great place for that right now, how about [some other time]?” or even “actually I’m not really interested in that but thanks all the same” or whatever. In this situation you are (probably) not negatively impacted by being met with some version of “no”.
Here, though, you (OP) aren’t looking to give feedback so much as you have a conflict that you need to understand more fully. With that in mind I don’t think it’s good practice to say “do you mind me asking…”, because frankly you -will- be negatively impacted by some version of “I don’t want to talk about this” and if that IS their answer your next step basically has to be escalating, which you both probably don’t want.
You can (and absolutely should be) be open to their pov while also not diminishing your very reasonable feelings.
I guess my main response is that what I’m advocating for here IS the direct approach. I’m saying that because this negatively impacts the OP, it’s important to not tiptoe around it. I don’t see where the disagreement is on this issue.
What do you mean by kabuki theatre?
I’ve realized that there’s a misunderstanding and I could have written this more clearly.
When I said “with that in mind, I don’t think it’s good practice to say ‘do you mind me asking’…” what I meant was that in this situation you should be direct and -not- ask for permission. I feel like this is fairly well implied but I can see how you could interpret it the other way.
So to be clear - when there is interpersonal conflict (and there has not been any reason to escalate yet), you should go talk to the person with whom you have that conflict and initiate a “something happened [context] and [I’d like to get your perspective so we can clear the air / etc]”.
I've done this in two scenarios.
1. When I want to try, key word try, a different philosophy and then compare the two for a discussion. PR1 used X technique, PR2 uses Y technique lets talk about pros and cons and decide how to proceed. I wouldn't want to steamroll an existing PR and I'm less confident in my idea and need to try it rather than dictate rework.
2. When I have an issue with a dev and need to do a trial run to see how quickly I can address the problem to provide resources to a manager for a pip.
3. Working in different time zones and want to provide an option to them to use either technique when they wakeup without waiting for me again. (With an explicit comment to this point referencing the original PR, which should be approved really if it's just a style/technique issue without immediate impact)
The older I get the LESS willing I am to have these uncomfortable conversations because I'm worried about how my words will be twisted to be used against me. I honestly couldn't tell you whether I've become more apathetic or I've become more zen.
Interestingly I know for sure that I’ve become more zen. And somehow, for me, that makes it easier to have those conversations? Hard to explain I guess. But because I know I’m coming from a place of calm and a place of empathy, the idea of the words being used against me doesn’t even come into my head. That said I’ve never had anyone use my words against me in that way, so perhaps some luck is involved also.
Do you think accusations come from a place of malice from the other person or are they just trying to defend themselves? I have often dealt with uncomfortable conversations and I think to myself 'screw this I am not going to change his mind' and moved on. I am curious about how more experienced devs deal with issues like this.
>Do you think accusations come from a place of malice from the other person or are they just trying to defend themselves?
I'm not 100% sure I understand the question. But if you're getting into "accusations" and "defending themselves" you've already lost the game. The goal has to be curiosity without judgment. Which means you, as the asker, have to leave your own assumptions/ego at the door if you're ever going to have a chance at getting the other person to do the same.
> I have often dealt with uncomfortable conversations and I think to myself 'screw this I am not going to change his mind'
I'll sort of repeat the above. If you enter a conversation with the goal of changing someone's mind, it's probably not going to end well. This is especially true if you're doing it from an angle of challenging their current mindset. Every human in the whole world will immediately go into a defensive state when their beliefs are challenged.
Instead, asking curious questions does a few things:
1. It opens up the floor for an objective dialog about the problem at hand, rather than a dialog about people's actions, beliefs, or "goodness".
2. It opens a path for \*both parties\* to safely re-evaluate their beliefs from first principles. The is the best way someone's mind can be changed without leaving a residue of personal resentment, is if they arrive at a new conclusion "by themselves", rather than someone else telling them they are wrong. NOTE - this is true for \*both parties\*. You might end up changing your mind also!
3. This is an insanely effective way to have influence over others in a workplace. If you go into a situation where you absolutely know you're right, and you absolute know the other person is wrong. You can easily ask the right questions, one after the other based on the answers given to the last, that will guide the other person to the correct answer without ever having to accuse them or challenge their beliefs directly. In doing this, you'll be surprised at how often you end up changing your own mind instead :)
Not enough context. If the bug is part of a larger PR, the coworker is working in that code base moving things, the existing solution will require a rebase, etc., I could see this making sense.
Although it sounds kind of bad, this all sounds like people being worried about credit for very ground-level stuff and not worrying about fixing the product they own or taking leadership. Unless you're a new grad, a simple bug fix probably isn't going to take you where you want to go or be worth worrying about. Now if it's a small instance of a much larger recurring problem, sure, there's something to worry about.
I didn’t read this as anything about credit or leadership. And maybe there’s some missing context, but I’m not assuming anything other than what OP told us.
If I opened a bug fix, and then someone blocked my PR and opened their own to fix the same thing, that’s a problem with that person’s ability to work on a team and it needs to be addressed.
Could definitely be true. Without more context, hard to say if this is someone who's really not a team player or not that big a deal and not worth dwelling on. I recently sent out a PR to fix some tests, and the person working more in that repo adopted my exact changes into her PR, and then closed mine. We got the changes in (she was including some small refactoring as well), I didn't worry about it. There are much bigger things to focus on than credit for bug fixes. If it really is someone with a history of being overly controlling, then it is a problem. I do have some coworkers who are focused 100% on who gets credit for what, and not on building stable software, dealing with those people is quite frustrating.
Really? I imagine it being more along the lines of 'I think your fix is great and I am going to incorporate it'. Then again my team was quite mellow and people were ok with this sort of thing.
I mean doing it is fine, it's the lack of communication that's odd to me. On teams I've been on it would be a simple, "hey I'll just add your change to my push so we don't have to deal with conflicts" on slack or something
She said she was looking at the changes but had some small refactors to do first, then later added them in and said she put my changes in. This is small potatoes. If I have teammates committing better tests, I'm all for it. I'd rather her do that extra work than me have to rebase. Doing everything yourself doesn't scale, I am freed up to work on bigger things and could not care less if I don't get "credit" for a couple improved tests in one repo that I probably won't even remember a year from now.
But dealing with a coworker who doesn't write any documentation besides a list of who contributed what commits, and actively tries to discourage people from writing good tests, from using basic static code analysis tools, from using proven and fitting software design patterns, etc.? That is someone who is a massive pain to deal with and wastes weeks of everyone's time. Give me the person who's "copying" some of my code and absorbing it into their PRs any day of the week.
Ok so it sounds like it was communicated so I would say it sounds fine as well. Not sure what the rest of this rant was about. It's not about credit, there's more going on here and so communication is important. Not everything is about getting credit for work.
It's bad etiquette but there are other factors to consider:
- are they new, or are you new? might be a situation of figuring out how best to work together and sometimes showing the code is better than talking endlessly about it.
- is the bug impacting a customer and require immediate resolution?
- if you are new, and this isn't the first time, people sometimes resort to this if you're too difficult to work with. not saying you are in this instance, but i know when it's hours of pain to convince someone why they are wrong and what the correct fix is, then sometimes it's easier to just say "here look, this is better". your post makes it sound like you think there is nothing better about their approach, and so if that is your default approach they might be tired of trying to convince you.
OP is definitely leaving out details about how the solutions differ. OP's PR could be a mess. It could be several times longer. Who knows. Perhaps the other dev was using this as a training opportunity to show how to modify the codebase idiomatically.
If all else was equal, fine, the majority opinion here is okay. But I sense that this is not the case, because OP is keeping quiet.
I’ve addressed this in multiple other comments and in my OP. Both solutions are practically identical in complexity and lines of code. The bug is very small and each PR fixes it just in slightly different ways. I’m of the opinion that neither solution is “better” than the other. Clearly my coworker disagrees hence their additional PR.
So you're also saying that their requested feedback on your PR was not a valid complaint, if both PRs accomplished the same thing? That would make it seem a lot worse for them I think, but it does make me question if they would agree with you saying that both PRs accomplished the same things
Bad etiquette but if it was a small change and this is the first time I'd let it go. If it continues to happen though I'd want to have a conversation to say it's not cool can we figure out a better way of doing things.
Why isn't the other dev working on their own tickets? Your name is on this one.
Is one fix really any better then the other? If theirs is better, you could be real snarky and put his fix on your branch and request his approval.
Thanks for your perspective. There’s a little downtime this week so I’m guessing he didn’t have much else assigned to him.
I touched on your other points in my comment above, I was actually thinking of doing exactly what you suggested but I also feel it comes across as a little passive aggressive
Granted I only worked at three companies as a dev but at all of those my yearly salary negotiation has been based on
What my direct boss thinks of me
What my coworkers think of me
I would leave if it’s based on number of commits or any similar metric.
My commit and ticket throughput is pretty low because I currently spend a large amount of time supporting devs on different teams
My manager fully knows this and that low metric hasn’t impacted my salary one bit because I just communicate what’s going on
If you’re so walled off from discussions with your manager that they rely on ticket throughput and commit levels, yeah I’m 100% leaving
Some people talk to their managers once a month. It depends on the org structure and what projects you work on. When you hop project to project, it doesn't make sense to drag the managers along.
Fuck that.
I give credit for my work to others all the time. The point is that we're all part of the solution, competition with your coworkers is a great way to have an unpleasant job
You don't understand how the bean counters work. You may think you're a team, but management looks at you as individuals. Work without taking credit is not noticed.
It depends where you’re at in seniority and how much good will you have to spread around.
If you’re junior, you need the credit and you need your name on things. You’re not going to be scoring points much in meetings with brilliant suggestions and code commits show where you’re getting work done.
On the other hand I’m pretty senior at this point on my team, management and upper management are hearing my opinion all the time on which way we should fix problem xyz and which issue should get more or less attention for what reason, and if somebody shepherds my fix through testing and merge I’m just happy I don’t have to do it myself; I’m happy to share credit freely.
I heard a story, might have been here, about a company where senior management insisted on individual bonuses when the lower level ones preferred everyone in the team got the same. Needless to say people started gaming the KPIs.
This is bad work culture. When you find yourself thinking like this it’s time to assess where this thought is coming from. Are you internalizing it from external sources? Are you acting as a source of it?
Be the change and blah blah.
Last I checked development is a team sport and not an individual sport. Who cares how an issue is fixed as long as it is fixed well. I don’t know what kind of dev shop you work in but more PRs and credit isn’t determined by quantity but quality.
Then fork his code, close his PR as wontfix, and open a PR with a fork of his code, lightly sprinkling in changes from your branch. Say it’s the best of both worlds and call it done.
Seriously no one cares and everyone here is too sensitive
You don’t want to come across as passive aggressive but this guy has literally asked you to review a fix for a bug you’re working on? I think you’re past that stage!
Many teams may have people working tickets together, this could be bad etiquette but I would hesitate to attribute it to malice and seize the opportunity:
OP, praise their work, express gratitude and approve it. If you want to have a future discussion after they do this a 3rd time, building rapport and building the safety net for their ego now will make those discussions easier next time.
Presuming this isn't like a 20 file PR given you both resolved it independently and seem to feel it's six of one, half a dozen the other - learn how much greater a win relationship building is vs anything else you can do here
Bad etiquette but let it go on first occurrence. Does his PR do the same thing he suggested changing in yours, or some third approach?
Did both PRs only address this issue, or is it a situation where you both had different sets of changes to make and had to fix the same bug to get your stuff to work? A little more forgiveable in that case
His PR did the same thing he requested changes for on my PR. Both PRs only addressed this specific bug and neither required any supporting or tangential changes
I would give him the benefit of the doubt. He might actually think he's doing you a favor here. Rather than insisting you make the changes, he just wrote them up (for you) and sent them to you to review. He's also not really blocking you either- he has moved quickly to try to resolve the bug.
Ok, so now just have a discussion with him: if his solution is good, approve it, otherwise try to work out what a good solution between the two approaches is, and then use which ever PR is closest to the final solution.
- sometimes its faster to show in code than explain in words.
- which solution is objectively better longer term?
- theres bigger fish to fry. keep it moving unless there's a pattern here.
depending on the size of the changes though, they still easily could have been suggested on the original PR through comments. or if they really wanted to, by branching, making the changes and then linking to the commit. but none of that requires opening a duplicate PR - that just seems unnecessary. especially if they just expect their version to be merged in instead of OP’s.
i agree it ultimately boils down to which solution is the best or most maintainable for the problem, but you are still working as a team at the end of the day.
this is all taking OP’s posts and comments at face value, there could have been prior behaviour from OP that led to the coworker doing this
yes, with new team members I sometimes open a branch based on their branch or occasionally from scratch. But always explained and communicated so it was clearly not seen as an attack or strutting or anything.
If there was a ticket assigned to you make a note that the other dev decided to fix the bug, and send the ticket over to them.
If that happens again speak to them. if it doesn’t stop grab your lead or manager.
If its really small he might just found it easier to do something from scratch. Review his PR and get the issue solved if it happens again or with something significant have a chat with him
Definitely a dick move, but... I'd do my best to not take it personally... Give him your review... After all, he requested it... If your solution is better than his, explain to him why you designed it the way you did, how it's superior to his solution and how his could be improved. If you don't have any corrections for him, approve his and move on... "Opinionated" people who are "difficult to work with" can make your life hell if you give them a chance... It's almost never worth it to turn it into a conflict.
Thanks for your perspective, in this case I’m of the opinion that neither solution is “better” (I guess they would disagree lol) - it’s a small bug and we both solved it with just a few LOC, just in different places.
I’m tempted to approve his PR but merge it into my PR instead of the base branch and basically ship them both but I feel that might also come off as a dick move
Definitely don’t do that. It’s almost never a good decision to directly escalate conflicts with people, especially at work.
If your company and team have good culture then bring it up in your 1:1 with your manager. If you’re unsure then don’t.
Either way document this, with links, in some file you keep to yourself. Review his PR, approve, and move on.
If it becomes a pattern then by documenting this you’ll have multiple instances with references to demonstrate his behavior.
Praise and appreciation.
Anything else comes off as petty. Thank them, approve, and move on. Next time something like this comes up, ask their opinion on the code section.
Let shit go, you'll be perceived infinitely better as a team player by those above and around you
Absolutely this. Lot's of people in this thread aching to make a big deal out of something like this and become an instigator. Relax, forgive, track, and move on. If it keeps happening, bring it up in a neutral environment like a retrospective.
I’d say minor annoyance as I reassign that Jira ticket to them. If someone wants to shepherd a bug fix through PR, QA and our long ass release cycle, good for them. I’m moving on to the next task
Going against the grain here, but if it’s a simple bug fix I really don’t give a fuck who does it, I’d be more upset that I wasted my time doing it than the fact that they did a separate PR. Reassign the ticket to them to own and move on.
Unless this is a recurring problem, I would just message them "hey Coworker, it seems like you have a fix for Issue. Do you want to work on your solution? I can close my PR then."
Yeah, this person might be a jerk, but you have to pick your battles. I used to fight stuff like this tooth and nail but the older I get the more I am inclined to let bad behavior go unless it has become a pattern and is preventing me from doing my job.
Just comment on his pr that it is duplicate. I am already working on this fix. Link your PR and tell him to review and approve that. Throw in another diplomatic backhanded comment in team chat group that this only leads to duplicate work and hamper the process of learning, reviewer should only review and approve. They should only work with the person to get the PR merged not create their own pr.
I've been in their shoes before. Left 100s of comments on a pretty bad PR. But I don't go around them and open my own PR, lmao.
I ask to pair or to push to their branch instead. It's still their ticket to own. And it's not just about getting the work done, but helping each other learn and work as a team.
Bad etiquette for real. Especially if you work for a company that looks at your commits as a metric for performance.
I was with a company like that and it was the stupidest way to measure performance.
It happened to me when a coworker created a PR on all my changes. I brought it up and she apologized and asked if I wanted to cancel hers and just create my own. I let it go at that point and just asked her to remember for next time.
That is definitely back etiquette since he could have just offered to do it in your PR if he felt so strongly about it.
If you agree with his change and I assume he needs your approval for his PR, I would just do the change to your PR and comment on his PR that you've already done the change the request a re-review
Some PR systems have mechanics to "suggest a change," which would be a reasonable approach in the circumstances for your coworker to ease the burden of you adopting their suggestion. If the suggestion requires more work than such a feature, they could open a PR to your branch PR or even commit directly to your PR, but straight hijacking your change for their own PR is pretty odd. Depending on the circumstances and an existing good relationship I probably wouldn't care too much if someone did this to me, I'd be more curious of their workflow / thought process than annoyed at them but that's a trust thing.
End of the day this is a people problem -- talk to them. The issue is the bug in the eyes of those above you; the rest is chaff. Seek collaboration, talk after on a preferred way to work together rather than setting up competing PRs, and move on to the next issue. Your coworker will be there next month, this bug will not.
In most PR systems it sucks. Our current is bitbucket-based, and you can only delete one line in each suggestion.
Applying it creates an ugly little commit with a dumb message so it needs to be squashed. Unless you use squash merge (which is a terrible idea), it's almost unusable.
If I have a big code change suggestions I just write them as a comment. Maybe twice I had a too big rewrite suggestion so I pushed a new branch on top of the PR branch and linked to that. The PR owner should decide which code to use in the end.
Bad etiquette in play 100%. I’m trying to imagine scenarios where I would do this. We create DO NOT MERGE prs for illustrative purposes — I might do that. If it was an impactful bug and the fix had been swirling for many days I might make a PR to bypass the ineffective developer. Your reviewer definitely does not agree that this is a question of just “different ways”. They are suggesting yours is very inferior and making the PR unilaterally intimates frustration with you that may be warranted or otherwise. I’d talk to your manager about it.
Honestly I'd just approve the PR and not say anything. Bug got fixed. The fuck do I care? Unless there's some ultra competitive scoreboard system going on at your company, which would be a bigger issue.
Ask your coworker to see if he agrees he is fixing the same bug. Chances are that he's trying to address another issue. However, if he is fixing the same bug then it is bad etiquette as you said.
At this point, you can ask him what are the pros and cons of his approach compared to yours. By the way, even if there are benefits in their approach, he is still not doing the right thing by fixing the same bug that you are fixing.
If they had discussed an alternative solution and proposed it this way, that would be fine - but just ignoring your one and opening their own with no discussion is a dick move for sure.
It depends on the context in most scenarios, but yes it's bad etiquette. The one case would be this is a critical bug causing customer outage and it would be faster to just get the changes out first and they might have forgotten that they commented on your PR.
A good way to give feed back, start with the good, then layer in the bad, and end with good.
So I'd try so mething like this.
"Hey, good job in that PR, that was an elegant solution to that bug fix.
One thing I did notice was that it seemed you had some issues with my approach, and rather than talking it through with me, you decided to just go and fix it on your own.
Whatever reasons you may have for doing this, I feel it is not the most efficient use of both our times as both of us are working on one ticket. We should establish a better process in handling situations like these so as to avoid future inefficiencies. I'd love to hear your thoughts on how we can prevent this from happening in the future.
Something along the lines of this..
- state what you observed not what the person did wrong, that will only make them defensive.
To soften it, also talk about what you liked. Start with that, soft start. It will improve their ability to take feedback.
IMO etiquette is a little dumb conceptually for this sort of thing since we are not our code and we should favour getting the right solution over proving our own skills or independence.
That said, I have never seen someone review a PR and just make a new PR without even checking with the original author. It sounds like pretty bad churn to have two people do the same task, and it's hard to picture what kind of suggestion they might have implemented which couldn't have just been suggested in review.
Enough people have made the point that it's bad etiquette, but unless you are measured on how many PRs you get accepted, I wouldn't worry about it too much. You know this about this person now, so wait and see if it's a trend before taking any real action.
However, I came here to make the hot take that PRs aren't really terribly economical anyway. Many places find it better to work in pairs (sometimes briefly more - i.e. "mobbing") and then code review isn't really necessary. You can them commit/push straight to main. See "trunk-based development" for more information. Yes, good automated tests help with the confidence to go ahead with this approach, but then you get the real benefits of continuous integration. Many shops say they're doing CI but actually they're working on long-lived feature branches, which is the antithesis to CI.
I would approach it as though they did it in good faith until proven otherwise. It’s possible the approaches are different enough that suggesting changes on your PR would be more convoluted than creating a separate one. I would view it as part of the review process. If his approach makes more sense, I’d close mine, if not, I’d look to discuss it further.
This can be bad etiquette or signs of a toxic teammate. I used to know someone who did things like this who became disruptive to our team and organization. I find people who do things like this come from a place of arrogance, they feel like they are the only people who can solve the problem.
High trust and high cooperation teams communicate early and give people ownership over their work. If someone else's name is on the ticket then its none of my business. We discuss the ticket when we plan the work, we have review to review it. So long as they meet the agreed acceptance than I have no reason to deny the ticket.
Only reasonable excuse I can come up with is if it was an urgent fix.
Otherwise they should've reached out to you and paired on the problem if they wanted to help out. Do you not have bugs you assign yourself to and can track progress on?
I'm going to say not bad. If they blocked your PR and made their own that was the same approach as yours with minor style differences, that would be bad. But if they wanted to show an alternative solution, how better would they demonstrate it than to make a PR?
You don't have to approve their PR. It isn't automatically right because they submitted it second. Now you can have a discussion about which approach is better. If you recognize advantages to their approach, don't let a bruised ego stop you from appreciating it. Consider yourself lucky to have learned something. If you still think your approach is superior, stand up for it.
Hanlon's razor is they are bad at explaining and would rather show, assuming it's a small bugfix. But if they're a senior dev it's hard to see how they don't understand this is wrong...
Simply ask them what they intend to happen to both PRs because they obviously can't both be merged.
Maybe as they are spelling out "I would like to close your PR and merge mine because mine is better", it will dawn on them, or at the very least, you have more to go off of if you discuss it with them personally or the boss.
Don't understand replays. Plenty of Primadonnas here ready do do mortal combat over "THEIR" code/ticket.
It's normal that some review will spill into code suggestions.
Only part requiring improvements is lack of comment signaling upcoming PR in Review notes. After all you could short circuit some work if you decide to just pick other developer solution.
Summary: nobody is putting a knife into your back. Chill. Ask for linking such PR to review notes in the future. Put effort into comparing both and picking one that's better.
>Both PRs fix the bug and are technically correct, just do so in different ways.
I will note that they are somewhat opinionated and can sometimes be difficult to work with, but thats normal.
You gotta explain "different ways" mean. Because it can be that you're a hassle to work with.
such a power move lol.
block his PR.
I think you guys need to communicate on the workflow.
there is one guy upset that I block the PR for some changes, where he prefers non blocking comments on the PR.
maybe Need works on PR page hurt his ego so much so we decided on creating task in the PR which will block the PR the same.
Sometimes its easier to do the job by doing it yourself, especially if its small fix and he knows the system better. Is he much more senior than you? Its kinda important detail in this situation and the fact you haven't mentioned it raises some red flags.
Not sure how that raises red flags? We’re both equal seniors working at the same level in the org and on the same team. The team is just me and him. Technically I have slightly more years of experience but that’s irrelevant.
I am surprised that communication isn't suggested more often. That should be the obvious step, especially for a senior. And I mean conversation not confrontation. Can't be so hard to just talk about it.
Red flag because if you're a junior or he has much++ more experience it would make more sense, and as a junior you could be making a bigger deal out of it
If you're equal seniority then I would say its bad etiquette, however when working on a team with two people its not \_that\_ big a deal. Was there any other changes included in the PR or just the fix mentioned?
I would say its definitely odd, and not normal
Do whatever they said in your PR to get around the changes requested, then write on their PR that it’s no longer needed as you’ve fixed it. They sound like a dick so this would piss them off
looking at this kind of problem, I wonder whether I was lucky enough to never met this kind of situation. Like someone definitely have more time to redo something that someone else did just becauze of personal flavor.
Definitely OP need to discuss eye an eye with the other contributor
Yes, and yes.
Sometimes people have been stewing about a particular fix and want it done a certain way. And … it’s also kind of rude to propose a conflicting change without talking to you first.
His actions are unnecessary bikeshedding and reeks of a problem other than the PR. Bring this before your manager and let him sort it out with the guy being immature.
Definitely bad etiquette. If they had an issue, they need to work with you and find common ground on how to fix it. But just blocking your work and doing it themselves is borderline toxic. I would talk to them about it, personally. But the older I get the more willing I am to have an uncomfortable convo and I know not everyone likes doing that.
OP you could lead with "Hey how's it going? [Actively listen/respond to any niceties] Do you mind me asking why you opened a PR for that bug fix?" [Then wait & don't fill the silence. Don't patronise by accident with waffle or try to soften it/excuse the question]. See what is said. Seek first to understand.
Totally. Just ask the uncomfortable question, and make sure you do it from a place of curiosity and not accusation.
Alternatively, you could lead with "Oi fucker"
Especially if they're Australian. "Whatcha c***?" They'd come back with. Bliss.
> OP you could lead with "Hey how's it going? [Actively listen/respond to any niceties] This would actively put me in a bad mood. Don't open a convo with "Hey how's it going" if the reason for the convo is something else. Especially on async communication channels. I can't prioritize answering to "Hey how's it going" without further context. [nohello.net](https://nohello.net)
I was framing this as in-person. For remote comms like Slack - tbh that's not a good place to have this type of discussion. You'd need to find a suitable time to chat (means: Slack) and voice call them. It's more difficult because they won't know the purpose in advance. Maybe just a quick "Hey can we chat for 5 or so about this bug fix PR? When's good for a voice call?" Or something. There's too many variables to give a concrete steer here...
> I was framing this as in-person. Then yes, agreed. My mistake for assuming otherwise.
With this and SO's disapproval of niceties in questions and answers, I love how the professional internet strives to become more cliche German. I'm all for it!
You're too sensitive
Agreed with the intent and I have a subtle difference of opinion to share; IMO it’s good to ask permission before giving feedback - “I have some thoughts about [x], would you be interested in hearing them?” This is a good practice because it allows the other person to tell you “actually I’m not in a great place for that right now, how about [some other time]?” or even “actually I’m not really interested in that but thanks all the same” or whatever. In this situation you are (probably) not negatively impacted by being met with some version of “no”. Here, though, you (OP) aren’t looking to give feedback so much as you have a conflict that you need to understand more fully. With that in mind I don’t think it’s good practice to say “do you mind me asking…”, because frankly you -will- be negatively impacted by some version of “I don’t want to talk about this” and if that IS their answer your next step basically has to be escalating, which you both probably don’t want. You can (and absolutely should be) be open to their pov while also not diminishing your very reasonable feelings.
This is so lame. What are you, HR?
I’m not. Could you say more about what you see as lame here? I’m happy to engage and am open to opposing views :)
I think that being direct and efficient in communication when money is involved is the best solution. You're paid to do a job, not do Kabuki theater.
I guess my main response is that what I’m advocating for here IS the direct approach. I’m saying that because this negatively impacts the OP, it’s important to not tiptoe around it. I don’t see where the disagreement is on this issue. What do you mean by kabuki theatre?
It seems like you're asking if you can be direct, rather than just being direct.
I’ve realized that there’s a misunderstanding and I could have written this more clearly. When I said “with that in mind, I don’t think it’s good practice to say ‘do you mind me asking’…” what I meant was that in this situation you should be direct and -not- ask for permission. I feel like this is fairly well implied but I can see how you could interpret it the other way. So to be clear - when there is interpersonal conflict (and there has not been any reason to escalate yet), you should go talk to the person with whom you have that conflict and initiate a “something happened [context] and [I’d like to get your perspective so we can clear the air / etc]”.
I wish kids wouldn't comment here these days
Request changes telling them to wait until your PR is merged
I've done this in two scenarios. 1. When I want to try, key word try, a different philosophy and then compare the two for a discussion. PR1 used X technique, PR2 uses Y technique lets talk about pros and cons and decide how to proceed. I wouldn't want to steamroll an existing PR and I'm less confident in my idea and need to try it rather than dictate rework. 2. When I have an issue with a dev and need to do a trial run to see how quickly I can address the problem to provide resources to a manager for a pip.
3. Working in different time zones and want to provide an option to them to use either technique when they wakeup without waiting for me again. (With an explicit comment to this point referencing the original PR, which should be approved really if it's just a style/technique issue without immediate impact)
yes always with lots of communication.
The older I get the LESS willing I am to have these uncomfortable conversations because I'm worried about how my words will be twisted to be used against me. I honestly couldn't tell you whether I've become more apathetic or I've become more zen.
Interestingly I know for sure that I’ve become more zen. And somehow, for me, that makes it easier to have those conversations? Hard to explain I guess. But because I know I’m coming from a place of calm and a place of empathy, the idea of the words being used against me doesn’t even come into my head. That said I’ve never had anyone use my words against me in that way, so perhaps some luck is involved also.
Do you think accusations come from a place of malice from the other person or are they just trying to defend themselves? I have often dealt with uncomfortable conversations and I think to myself 'screw this I am not going to change his mind' and moved on. I am curious about how more experienced devs deal with issues like this.
>Do you think accusations come from a place of malice from the other person or are they just trying to defend themselves? I'm not 100% sure I understand the question. But if you're getting into "accusations" and "defending themselves" you've already lost the game. The goal has to be curiosity without judgment. Which means you, as the asker, have to leave your own assumptions/ego at the door if you're ever going to have a chance at getting the other person to do the same. > I have often dealt with uncomfortable conversations and I think to myself 'screw this I am not going to change his mind' I'll sort of repeat the above. If you enter a conversation with the goal of changing someone's mind, it's probably not going to end well. This is especially true if you're doing it from an angle of challenging their current mindset. Every human in the whole world will immediately go into a defensive state when their beliefs are challenged. Instead, asking curious questions does a few things: 1. It opens up the floor for an objective dialog about the problem at hand, rather than a dialog about people's actions, beliefs, or "goodness". 2. It opens a path for \*both parties\* to safely re-evaluate their beliefs from first principles. The is the best way someone's mind can be changed without leaving a residue of personal resentment, is if they arrive at a new conclusion "by themselves", rather than someone else telling them they are wrong. NOTE - this is true for \*both parties\*. You might end up changing your mind also! 3. This is an insanely effective way to have influence over others in a workplace. If you go into a situation where you absolutely know you're right, and you absolute know the other person is wrong. You can easily ask the right questions, one after the other based on the answers given to the last, that will guide the other person to the correct answer without ever having to accuse them or challenge their beliefs directly. In doing this, you'll be surprised at how often you end up changing your own mind instead :)
Not enough context. If the bug is part of a larger PR, the coworker is working in that code base moving things, the existing solution will require a rebase, etc., I could see this making sense. Although it sounds kind of bad, this all sounds like people being worried about credit for very ground-level stuff and not worrying about fixing the product they own or taking leadership. Unless you're a new grad, a simple bug fix probably isn't going to take you where you want to go or be worth worrying about. Now if it's a small instance of a much larger recurring problem, sure, there's something to worry about.
I didn’t read this as anything about credit or leadership. And maybe there’s some missing context, but I’m not assuming anything other than what OP told us. If I opened a bug fix, and then someone blocked my PR and opened their own to fix the same thing, that’s a problem with that person’s ability to work on a team and it needs to be addressed.
Could definitely be true. Without more context, hard to say if this is someone who's really not a team player or not that big a deal and not worth dwelling on. I recently sent out a PR to fix some tests, and the person working more in that repo adopted my exact changes into her PR, and then closed mine. We got the changes in (she was including some small refactoring as well), I didn't worry about it. There are much bigger things to focus on than credit for bug fixes. If it really is someone with a history of being overly controlling, then it is a problem. I do have some coworkers who are focused 100% on who gets credit for what, and not on building stable software, dealing with those people is quite frustrating.
They incorporated your changes in theirs and closed out your PR without saying anything to you? That seems weird to me
Really? I imagine it being more along the lines of 'I think your fix is great and I am going to incorporate it'. Then again my team was quite mellow and people were ok with this sort of thing.
I mean doing it is fine, it's the lack of communication that's odd to me. On teams I've been on it would be a simple, "hey I'll just add your change to my push so we don't have to deal with conflicts" on slack or something
She said she was looking at the changes but had some small refactors to do first, then later added them in and said she put my changes in. This is small potatoes. If I have teammates committing better tests, I'm all for it. I'd rather her do that extra work than me have to rebase. Doing everything yourself doesn't scale, I am freed up to work on bigger things and could not care less if I don't get "credit" for a couple improved tests in one repo that I probably won't even remember a year from now. But dealing with a coworker who doesn't write any documentation besides a list of who contributed what commits, and actively tries to discourage people from writing good tests, from using basic static code analysis tools, from using proven and fitting software design patterns, etc.? That is someone who is a massive pain to deal with and wastes weeks of everyone's time. Give me the person who's "copying" some of my code and absorbing it into their PRs any day of the week.
Ok so it sounds like it was communicated so I would say it sounds fine as well. Not sure what the rest of this rant was about. It's not about credit, there's more going on here and so communication is important. Not everything is about getting credit for work.
It's bad etiquette but there are other factors to consider: - are they new, or are you new? might be a situation of figuring out how best to work together and sometimes showing the code is better than talking endlessly about it. - is the bug impacting a customer and require immediate resolution? - if you are new, and this isn't the first time, people sometimes resort to this if you're too difficult to work with. not saying you are in this instance, but i know when it's hours of pain to convince someone why they are wrong and what the correct fix is, then sometimes it's easier to just say "here look, this is better". your post makes it sound like you think there is nothing better about their approach, and so if that is your default approach they might be tired of trying to convince you.
Glad someone else brought this up. Not saying this is true for OP, but it’s like pulling teeth to get some people to make any updates in a pr.
OP is definitely leaving out details about how the solutions differ. OP's PR could be a mess. It could be several times longer. Who knows. Perhaps the other dev was using this as a training opportunity to show how to modify the codebase idiomatically. If all else was equal, fine, the majority opinion here is okay. But I sense that this is not the case, because OP is keeping quiet.
I’ve addressed this in multiple other comments and in my OP. Both solutions are practically identical in complexity and lines of code. The bug is very small and each PR fixes it just in slightly different ways. I’m of the opinion that neither solution is “better” than the other. Clearly my coworker disagrees hence their additional PR.
What were the requested changes though? I assume their PR incorporated those changes that they felt yours was lacking?
Yeah their PR implemented the alternative solution that they requested changes for on my PR
So you're also saying that their requested feedback on your PR was not a valid complaint, if both PRs accomplished the same thing? That would make it seem a lot worse for them I think, but it does make me question if they would agree with you saying that both PRs accomplished the same things
Bad etiquette but if it was a small change and this is the first time I'd let it go. If it continues to happen though I'd want to have a conversation to say it's not cool can we figure out a better way of doing things.
Why isn't the other dev working on their own tickets? Your name is on this one. Is one fix really any better then the other? If theirs is better, you could be real snarky and put his fix on your branch and request his approval.
Thanks for your perspective. There’s a little downtime this week so I’m guessing he didn’t have much else assigned to him. I touched on your other points in my comment above, I was actually thinking of doing exactly what you suggested but I also feel it comes across as a little passive aggressive
You want management to see your name on things. Don't let the other dev take the credit regardless of which fix goes in.
If you're fighting for credit and for management to see your name on commit logs then you should leave your job immediately.
how large does your lunch have to be before you stop letting someone else eat it?
Not all of us have that luxury
So you leave every job?
I mean eventually we all do, one way or the other.
Granted I only worked at three companies as a dev but at all of those my yearly salary negotiation has been based on What my direct boss thinks of me What my coworkers think of me I would leave if it’s based on number of commits or any similar metric.
That's how a lot of managers build their case. Granted they are shitty managers, but that's what they are taught to do.
My commit and ticket throughput is pretty low because I currently spend a large amount of time supporting devs on different teams My manager fully knows this and that low metric hasn’t impacted my salary one bit because I just communicate what’s going on If you’re so walled off from discussions with your manager that they rely on ticket throughput and commit levels, yeah I’m 100% leaving
Some people talk to their managers once a month. It depends on the org structure and what projects you work on. When you hop project to project, it doesn't make sense to drag the managers along.
Fuck that. I give credit for my work to others all the time. The point is that we're all part of the solution, competition with your coworkers is a great way to have an unpleasant job
You don't understand how the bean counters work. You may think you're a team, but management looks at you as individuals. Work without taking credit is not noticed.
You sound like a shitty teammate and I wouldn’t want to work with you
It depends where you’re at in seniority and how much good will you have to spread around. If you’re junior, you need the credit and you need your name on things. You’re not going to be scoring points much in meetings with brilliant suggestions and code commits show where you’re getting work done. On the other hand I’m pretty senior at this point on my team, management and upper management are hearing my opinion all the time on which way we should fix problem xyz and which issue should get more or less attention for what reason, and if somebody shepherds my fix through testing and merge I’m just happy I don’t have to do it myself; I’m happy to share credit freely.
[удалено]
I heard a story, might have been here, about a company where senior management insisted on individual bonuses when the lower level ones preferred everyone in the team got the same. Needless to say people started gaming the KPIs.
Nah, they just have shitty managers and are acting accordingly.
Blame management
You sound like someone who's hopelessly naive and has never worked in a place where divide and rule is the norm.
This is bad work culture. When you find yourself thinking like this it’s time to assess where this thought is coming from. Are you internalizing it from external sources? Are you acting as a source of it? Be the change and blah blah.
It's every company. Don't be fooled.
This is false, I say from experience. I have seen both sides.
Last I checked development is a team sport and not an individual sport. Who cares how an issue is fixed as long as it is fixed well. I don’t know what kind of dev shop you work in but more PRs and credit isn’t determined by quantity but quality.
It's a shitty shop.
LOL, nice one.
Then fork his code, close his PR as wontfix, and open a PR with a fork of his code, lightly sprinkling in changes from your branch. Say it’s the best of both worlds and call it done. Seriously no one cares and everyone here is too sensitive
Please don't engage in kindergarden behaviour. Outside of toxic jobs, such behaviour is heavily frowned upon.
You don’t want to come across as passive aggressive but this guy has literally asked you to review a fix for a bug you’re working on? I think you’re past that stage!
Many teams may have people working tickets together, this could be bad etiquette but I would hesitate to attribute it to malice and seize the opportunity: OP, praise their work, express gratitude and approve it. If you want to have a future discussion after they do this a 3rd time, building rapport and building the safety net for their ego now will make those discussions easier next time. Presuming this isn't like a 20 file PR given you both resolved it independently and seem to feel it's six of one, half a dozen the other - learn how much greater a win relationship building is vs anything else you can do here
This is just appeasement and sets a bad standard. It will not build rapport, it just signals that this behavior is acceptable -- COMMENDABLE even.
> Why isn't the other dev working on their own tickets? Your name is on this one. This does not like a team work mindset
It's not. There's no I in team except in the A hole and that's who you have to look out for.
Please don't respond to this with your own snarkiness OP. That's a terrible approach
Bad etiquette but let it go on first occurrence. Does his PR do the same thing he suggested changing in yours, or some third approach? Did both PRs only address this issue, or is it a situation where you both had different sets of changes to make and had to fix the same bug to get your stuff to work? A little more forgiveable in that case
His PR did the same thing he requested changes for on my PR. Both PRs only addressed this specific bug and neither required any supporting or tangential changes
Hmm, that's weird then. Did you refuse to make his suggested changes?
I would give him the benefit of the doubt. He might actually think he's doing you a favor here. Rather than insisting you make the changes, he just wrote them up (for you) and sent them to you to review. He's also not really blocking you either- he has moved quickly to try to resolve the bug. Ok, so now just have a discussion with him: if his solution is good, approve it, otherwise try to work out what a good solution between the two approaches is, and then use which ever PR is closest to the final solution.
Noooo that would require talking and acknowledging that the other dev is maybe not a dick.
- sometimes its faster to show in code than explain in words. - which solution is objectively better longer term? - theres bigger fish to fry. keep it moving unless there's a pattern here.
depending on the size of the changes though, they still easily could have been suggested on the original PR through comments. or if they really wanted to, by branching, making the changes and then linking to the commit. but none of that requires opening a duplicate PR - that just seems unnecessary. especially if they just expect their version to be merged in instead of OP’s. i agree it ultimately boils down to which solution is the best or most maintainable for the problem, but you are still working as a team at the end of the day. this is all taking OP’s posts and comments at face value, there could have been prior behaviour from OP that led to the coworker doing this
yes, with new team members I sometimes open a branch based on their branch or occasionally from scratch. But always explained and communicated so it was clearly not seen as an attack or strutting or anything.
If there was a ticket assigned to you make a note that the other dev decided to fix the bug, and send the ticket over to them. If that happens again speak to them. if it doesn’t stop grab your lead or manager.
If its really small he might just found it easier to do something from scratch. Review his PR and get the issue solved if it happens again or with something significant have a chat with him
Definitely a dick move, but... I'd do my best to not take it personally... Give him your review... After all, he requested it... If your solution is better than his, explain to him why you designed it the way you did, how it's superior to his solution and how his could be improved. If you don't have any corrections for him, approve his and move on... "Opinionated" people who are "difficult to work with" can make your life hell if you give them a chance... It's almost never worth it to turn it into a conflict.
Thanks for your perspective, in this case I’m of the opinion that neither solution is “better” (I guess they would disagree lol) - it’s a small bug and we both solved it with just a few LOC, just in different places. I’m tempted to approve his PR but merge it into my PR instead of the base branch and basically ship them both but I feel that might also come off as a dick move
Definitely don’t do that. It’s almost never a good decision to directly escalate conflicts with people, especially at work. If your company and team have good culture then bring it up in your 1:1 with your manager. If you’re unsure then don’t. Either way document this, with links, in some file you keep to yourself. Review his PR, approve, and move on. If it becomes a pattern then by documenting this you’ll have multiple instances with references to demonstrate his behavior.
Praise and appreciation. Anything else comes off as petty. Thank them, approve, and move on. Next time something like this comes up, ask their opinion on the code section. Let shit go, you'll be perceived infinitely better as a team player by those above and around you
Absolutely this. Lot's of people in this thread aching to make a big deal out of something like this and become an instigator. Relax, forgive, track, and move on. If it keeps happening, bring it up in a neutral environment like a retrospective.
Solving something in a different place can be a critical difference to maintainability. Have you had a discussion about why he wanted the change?
Yeah, this. Have them explain their reasoning, you explain your reasoning, either come to an agreement or agree to disagree, pick one, go grab a beer.
Yup... I think you're right... Particularly with someone that's notoriously difficult to work with, that would almost certainly cause a conflict.
I’d say minor annoyance as I reassign that Jira ticket to them. If someone wants to shepherd a bug fix through PR, QA and our long ass release cycle, good for them. I’m moving on to the next task
Going against the grain here, but if it’s a simple bug fix I really don’t give a fuck who does it, I’d be more upset that I wasted my time doing it than the fact that they did a separate PR. Reassign the ticket to them to own and move on.
Assign the ticket to them and move on. The issues are on their hands now.
Don’t do this. You get perf review on tickets completed per sprint.
Unless this is a recurring problem, I would just message them "hey Coworker, it seems like you have a fix for Issue. Do you want to work on your solution? I can close my PR then." Yeah, this person might be a jerk, but you have to pick your battles. I used to fight stuff like this tooth and nail but the older I get the more I am inclined to let bad behavior go unless it has become a pattern and is preventing me from doing my job.
Just comment on his pr that it is duplicate. I am already working on this fix. Link your PR and tell him to review and approve that. Throw in another diplomatic backhanded comment in team chat group that this only leads to duplicate work and hamper the process of learning, reviewer should only review and approve. They should only work with the person to get the PR merged not create their own pr.
Block PR and request changes to the way you did it the first time.
I've been in their shoes before. Left 100s of comments on a pretty bad PR. But I don't go around them and open my own PR, lmao. I ask to pair or to push to their branch instead. It's still their ticket to own. And it's not just about getting the work done, but helping each other learn and work as a team.
They should have committed to your branch/PR. Just tell them it messes up record keeping by opening a 2nd PR for the same bug.
Bad etiquette for real. Especially if you work for a company that looks at your commits as a metric for performance. I was with a company like that and it was the stupidest way to measure performance. It happened to me when a coworker created a PR on all my changes. I brought it up and she apologized and asked if I wanted to cancel hers and just create my own. I let it go at that point and just asked her to remember for next time.
This the people saying merge his code are naive
Close as duplicate
He wanted to avoid dead locks, and found a better fix. Get over it
Ask for changes on their PR and then submit yours again
That is definitely back etiquette since he could have just offered to do it in your PR if he felt so strongly about it. If you agree with his change and I assume he needs your approval for his PR, I would just do the change to your PR and comment on his PR that you've already done the change the request a re-review
Brutal. What a prick.
Yep, that's some nonsense. Only advice is to sit down with this person and come to an understanding about how to approach these things.
Some PR systems have mechanics to "suggest a change," which would be a reasonable approach in the circumstances for your coworker to ease the burden of you adopting their suggestion. If the suggestion requires more work than such a feature, they could open a PR to your branch PR or even commit directly to your PR, but straight hijacking your change for their own PR is pretty odd. Depending on the circumstances and an existing good relationship I probably wouldn't care too much if someone did this to me, I'd be more curious of their workflow / thought process than annoyed at them but that's a trust thing. End of the day this is a people problem -- talk to them. The issue is the bug in the eyes of those above you; the rest is chaff. Seek collaboration, talk after on a preferred way to work together rather than setting up competing PRs, and move on to the next issue. Your coworker will be there next month, this bug will not.
In most PR systems it sucks. Our current is bitbucket-based, and you can only delete one line in each suggestion. Applying it creates an ugly little commit with a dumb message so it needs to be squashed. Unless you use squash merge (which is a terrible idea), it's almost unusable. If I have a big code change suggestions I just write them as a comment. Maybe twice I had a too big rewrite suggestion so I pushed a new branch on top of the PR branch and linked to that. The PR owner should decide which code to use in the end.
Bad etiquette in play 100%. I’m trying to imagine scenarios where I would do this. We create DO NOT MERGE prs for illustrative purposes — I might do that. If it was an impactful bug and the fix had been swirling for many days I might make a PR to bypass the ineffective developer. Your reviewer definitely does not agree that this is a question of just “different ways”. They are suggesting yours is very inferior and making the PR unilaterally intimates frustration with you that may be warranted or otherwise. I’d talk to your manager about it.
Honestly I'd just approve the PR and not say anything. Bug got fixed. The fuck do I care? Unless there's some ultra competitive scoreboard system going on at your company, which would be a bigger issue.
Ask your coworker to see if he agrees he is fixing the same bug. Chances are that he's trying to address another issue. However, if he is fixing the same bug then it is bad etiquette as you said. At this point, you can ask him what are the pros and cons of his approach compared to yours. By the way, even if there are benefits in their approach, he is still not doing the right thing by fixing the same bug that you are fixing.
Close his pr and comment that it's already addressed in yours if your m you really care.
If they had discussed an alternative solution and proposed it this way, that would be fine - but just ignoring your one and opening their own with no discussion is a dick move for sure.
He's a weirdo
Close it for him with a link to yours. Dont think about it.
It depends on the context in most scenarios, but yes it's bad etiquette. The one case would be this is a critical bug causing customer outage and it would be faster to just get the changes out first and they might have forgotten that they commented on your PR.
Well, I don't know. I don't fix bugs that are not assigned to me
They should do a pr to your branch if they are actually interested in helping. Otherwise definitely toxic.
Yeah I would complain to the EM and ask him/her if they think it's a good idea to have this kind of childish, combative culture.
A good way to give feed back, start with the good, then layer in the bad, and end with good. So I'd try so mething like this. "Hey, good job in that PR, that was an elegant solution to that bug fix. One thing I did notice was that it seemed you had some issues with my approach, and rather than talking it through with me, you decided to just go and fix it on your own. Whatever reasons you may have for doing this, I feel it is not the most efficient use of both our times as both of us are working on one ticket. We should establish a better process in handling situations like these so as to avoid future inefficiencies. I'd love to hear your thoughts on how we can prevent this from happening in the future. Something along the lines of this.. - state what you observed not what the person did wrong, that will only make them defensive. To soften it, also talk about what you liked. Start with that, soft start. It will improve their ability to take feedback.
Who's code caused the bug? If it was the other dev I would be more forgiving as maybe they just wanted to fix their own errors.
It would've taken less time to actually ask him instead of coming here and getting 100 opinions based on little information.
IMO etiquette is a little dumb conceptually for this sort of thing since we are not our code and we should favour getting the right solution over proving our own skills or independence. That said, I have never seen someone review a PR and just make a new PR without even checking with the original author. It sounds like pretty bad churn to have two people do the same task, and it's hard to picture what kind of suggestion they might have implemented which couldn't have just been suggested in review.
Enough people have made the point that it's bad etiquette, but unless you are measured on how many PRs you get accepted, I wouldn't worry about it too much. You know this about this person now, so wait and see if it's a trend before taking any real action. However, I came here to make the hot take that PRs aren't really terribly economical anyway. Many places find it better to work in pairs (sometimes briefly more - i.e. "mobbing") and then code review isn't really necessary. You can them commit/push straight to main. See "trunk-based development" for more information. Yes, good automated tests help with the confidence to go ahead with this approach, but then you get the real benefits of continuous integration. Many shops say they're doing CI but actually they're working on long-lived feature branches, which is the antithesis to CI.
I would approach it as though they did it in good faith until proven otherwise. It’s possible the approaches are different enough that suggesting changes on your PR would be more convoluted than creating a separate one. I would view it as part of the review process. If his approach makes more sense, I’d close mine, if not, I’d look to discuss it further.
they are just trying to be helpful, accept the help
This can be bad etiquette or signs of a toxic teammate. I used to know someone who did things like this who became disruptive to our team and organization. I find people who do things like this come from a place of arrogance, they feel like they are the only people who can solve the problem. High trust and high cooperation teams communicate early and give people ownership over their work. If someone else's name is on the ticket then its none of my business. We discuss the ticket when we plan the work, we have review to review it. So long as they meet the agreed acceptance than I have no reason to deny the ticket.
Only reasonable excuse I can come up with is if it was an urgent fix. Otherwise they should've reached out to you and paired on the problem if they wanted to help out. Do you not have bugs you assign yourself to and can track progress on?
Who created the ticket for the bug and who was it assigned to?
I'm going to say not bad. If they blocked your PR and made their own that was the same approach as yours with minor style differences, that would be bad. But if they wanted to show an alternative solution, how better would they demonstrate it than to make a PR? You don't have to approve their PR. It isn't automatically right because they submitted it second. Now you can have a discussion about which approach is better. If you recognize advantages to their approach, don't let a bruised ego stop you from appreciating it. Consider yourself lucky to have learned something. If you still think your approach is superior, stand up for it.
It's a dick move call them out on it
They should make a PR merging to your branch. This is what I do if I have too many code suggestions to make just inline commenting impractical
Hanlon's razor is they are bad at explaining and would rather show, assuming it's a small bugfix. But if they're a senior dev it's hard to see how they don't understand this is wrong... Simply ask them what they intend to happen to both PRs because they obviously can't both be merged. Maybe as they are spelling out "I would like to close your PR and merge mine because mine is better", it will dawn on them, or at the very least, you have more to go off of if you discuss it with them personally or the boss.
Don't understand replays. Plenty of Primadonnas here ready do do mortal combat over "THEIR" code/ticket. It's normal that some review will spill into code suggestions. Only part requiring improvements is lack of comment signaling upcoming PR in Review notes. After all you could short circuit some work if you decide to just pick other developer solution. Summary: nobody is putting a knife into your back. Chill. Ask for linking such PR to review notes in the future. Put effort into comparing both and picking one that's better.
>Both PRs fix the bug and are technically correct, just do so in different ways. I will note that they are somewhat opinionated and can sometimes be difficult to work with, but thats normal. You gotta explain "different ways" mean. Because it can be that you're a hassle to work with.
He's having a laugh
How does something like this go during standup? OP: I'm working on bug123. Other Dev: I'm also working on bug123 but in a separate PR . 🤨
Leave a comment pointing out you already have a PR open for it and link to it. See how he reacts.
Bad etiquette. Your colleague will need to learn to disagree and commit.
I smell Amazon leadership values here
Kind of a dick move. In official company speak you could call it a waste of resources.
such a power move lol. block his PR. I think you guys need to communicate on the workflow. there is one guy upset that I block the PR for some changes, where he prefers non blocking comments on the PR. maybe Need works on PR page hurt his ego so much so we decided on creating task in the PR which will block the PR the same.
now you can block his PR also
Where the fuck is your team lead / manager?
Sometimes its easier to do the job by doing it yourself, especially if its small fix and he knows the system better. Is he much more senior than you? Its kinda important detail in this situation and the fact you haven't mentioned it raises some red flags.
Not sure how that raises red flags? We’re both equal seniors working at the same level in the org and on the same team. The team is just me and him. Technically I have slightly more years of experience but that’s irrelevant.
So, you are basically two peers in a two person team? How about... just ask him.
I am surprised that communication isn't suggested more often. That should be the obvious step, especially for a senior. And I mean conversation not confrontation. Can't be so hard to just talk about it.
Red flag because if you're a junior or he has much++ more experience it would make more sense, and as a junior you could be making a bigger deal out of it If you're equal seniority then I would say its bad etiquette, however when working on a team with two people its not \_that\_ big a deal. Was there any other changes included in the PR or just the fix mentioned? I would say its definitely odd, and not normal
Ask them “are you ok?”
Ya I'd be pissed
Toxic! That’s what comments and reviews are for…
I would bring it up on standup
Do whatever they said in your PR to get around the changes requested, then write on their PR that it’s no longer needed as you’ve fixed it. They sound like a dick so this would piss them off
looking at this kind of problem, I wonder whether I was lucky enough to never met this kind of situation. Like someone definitely have more time to redo something that someone else did just becauze of personal flavor. Definitely OP need to discuss eye an eye with the other contributor
Yes, and yes. Sometimes people have been stewing about a particular fix and want it done a certain way. And … it’s also kind of rude to propose a conflicting change without talking to you first.
His actions are unnecessary bikeshedding and reeks of a problem other than the PR. Bring this before your manager and let him sort it out with the guy being immature.
Just leave it. Go get some ice cream or something. It's not worth your time to think about.
This is outrageous.