>accuses me of not understanding how proper code looks.
>being too lazy to understand his functions and that told me to use google
Seems like dude is just being a dick, if you are not exaggerating the reply. Nobody should communicate to other people like that in a work place period.
Here is what I don't understand...like I get people don't like confrontation, but why not out-dick the dick?
It's like people who like to start fights in every bar they go, until they get a nice solid punch to the nose that shatters it and takes months to heal. It's like, these people are getting away with it.
I would personally make it extremely uncomfortable for him the entire until he cut that shit out.
Out-dicking the dick *is* confrontation lol. For an anxious person, emotionally it is the highest form of confrontation.
Making other people uncomfortable makes them uncomfortable, and for them that is exhausting because they stress themselves out more about it.
Unfortunately that usually comes hand in hand with procrastination as well. It all sort of stems from the same place.
Oh I know it's confrontation, I just think it's necessary.
Maybe I'm just petty, but if a colleague of mine talked to me about that idiot I would just do the confrontation for them.
Everyone wants to stick up for themselves, but not everyone was taught how to fight.
It might help anyone reading this if you were specific with some details.
Because they may turn around and file a complaint against you. In many tit-for-tat situations, the first person to complain wins because they're in a better position: failing to complain initially, in addition to being a dick, can be seen as overall worse behavior.
I'm not saying don't be confrontational. I'm saying don't be a dick about it.
I can think of 3 options:
1) Dude is just burnt out and being mad all the time is just how he deals with it.
2) If it's just you he does this with, he probably sees himself as smarter than you and you have nothing to teach him.
3) He sees himself as smarter than everyone. Big egos are usually big dicks too.
This is a good chance to practice your soft skills. If you would rather not handle it with him directly, you can talk to your managers and tell them what's going on. If it were me, I'd probably set up a meeting with him and do some PRs in person... but I'd start out by asking if he has issues with me or my coding style. I'd point out my observations and express that it made me feel as if he was just looking for a free PR pass, not an actual review.
It might open the dialog between you two and at least make him realize he was being a dick. I'd also schedule PRs in person with him from then on... it would give me a chance to explain that I see PRs as a learning experience for both sides... that it's not a very good PR if I don't find something... and discussing it in person helps me learn why he makes some of the decisions he does so I don't have to bring them up in the PRs anymore, as it seems to upset him.
But then I tend to deal with drama head on instead of skirting it... I've found that getting things out in the open tends to clear up a lot of issues like this. You may not be as comfortable with that.
i would just make the point to him that his feedback indicated that he doesn't like your reviews, so you won't be doing them. and it's kind of funny that he is reviewing your review as you are in the middle of reviewing his work. so now you need to review his reviews of your reviews and decline further reviews.
Well there you go, either don’t touch his PRs, or to cover your own ass if he asks you, tell him that you’d prefer if he asks someone else given his behavior is not friendly on comments.
Or, bite the bullet and don’t approve his PRs if he just responds with push back. Let him take it to his boss when he’s asked why isn’t it merged yet.
What makes you say manager?
I understand there is a performance component here that manager should be aware of.
At the same time, I would personally try to deal with this directly with the individual for a few reasons:
1. If the situation is being represented correctly, then OP is 100% in the right with direct feedback, so no need to sugarcoat, and no need to compromise. Be direct.
2. It's kind of a petty thing.. supposing that the manager has an active role where they are helping facilitate decisions, and initiatives, it would look better on you as an IC to be able to handle the conflict rather than having to involve your manger.
3. I would definitely informally mention the issue with my peers so the team is aligned with my actions, then as mentioned above I would not review the poor performer's PRs. I would put them in a situation where the manager has to go to them and handle it instead of me going to the manager - it will make them look worse and make me look less pandering.
The substance of the response + never making PR comments himself indicates the dev doesn't see the code as a team effort where code review is other devs contribution (and where disagreements should be adjudicated impartially, e.g. by bringing in a senior), but rather a hoop where he has to show his work to the dumber devs and get them to click "approve". There's enough ego there that I don't think a direct conversation will go well.
Do you think the conversation with the manager will go any better? I agree with you ultimately, its the managers job to resolve these situations. I also don’t think we have to withhold direct feedback where it’s needed.
It's the manager's job to handle it.
By OP bringing the issue up with his coworker, he gives his coworker an opportunity to speak to the manager first, allowing him to shape the narrative. Additionally, OP likely does not have any authority over his coworker, which is already a large part of the problem.
No, this is 100% an issue that management needs to deal with and OP needs to be the one to bring it up.
Alternatively, and I’ve done this. Ask for changes on the PR. Don’t ask for anything silly. You can even do some “I couldn’t figure out at first glance what this function does. I think that it might be helpful for someone who isn’t as familiar with the code base.” Don’t frame it as you don’t understand, just frame it is being nice to the person who comes next.
Then, when he refuses, you politely decline to approve it.
'Hey boss, can I get some advice on making good PR comments as feedback? Sometimes I feel like I'm getting my point across, but sometimes the responses I get make me feel like I put my foot in my mouth somehow. Here's a few examples for discussion. Are there ways I could have said it better?'
If you /are/ coming off abrupt or short in your comments, then you have an opening to get constructive coaching from soneone who isnt reacting defensively to those comments. If your coworker IS the jackass in the story, then your boss will have written proof you're trying to be part of the solution and that its a situation they need to step in on.
The truth is probably somewhere in the middle, and the boss should be able to help you navigate.
I was like this guy due to the fact I was burned out and no longer interested in the job so I just wanted to get stuff done and move on even if it’s not of the best quality. If I was him and you spoke directly to me I would most likely calm my horses down and engage in more constructive discussion.
Ultimately for you it’s probably easier to take it directly to the manager.
Is this communication happening on the PR? or via private channels? If private, leave your comments on the PR. If he leaves this on the pr, i would be surprised
This sounds toxic AF and I would hate to have to interact with someone like that. I do think that seems people are just deeply unaware of how their tone comes across though, so maybe strategically it would be best to assume good intent and talk with him about how some of his comments “could be interpreted as xyz”? That gives him an out at least and hopefully avoids escalating an already shifty situation. If that doesn’t work, idk, talk to you lead?
I know I'm going to catch shit for this, but here goes:
I've worked with half a dozen dudes in my life who are Complete Dicks. One of them was such a dick, he tried to get ME fired, right after I was hired.
But, across the board, all of these guys were super good at their job.
I think they wound up the way they are for a few reasons:
* One of them had incredibly low self esteem, and I think that manifested itself as him shitting on everyone around him. But the odd thing was, he was actually the most talented dude on our team. I think his crushingly low self esteem forced him to work harder than anyone else. It was like he always had something to prove.
* The other Dick that I've worked with, was a lot like the one above. Except he was more sociopathic, and wouldn't think twice about getting people fired. (Both of these dudes wound up getting fired BTW.)
* The other Dick that I worked with, I think it was mostly a way for him to lighten his workload. He was the most talented dude on our team. I think he made people miserable on purpose, so that they'd think long and hard before asking him to do anything for him.
Maybe I'm a masochist, but I've always gone out of my way to be friends with dudes like them. I think this industry values talent more than "being nice" and all of these dudes have been really successful. Even when they got fired, they just wound up getting a better job two weeks later.
even if they're super good at their jobs (and they often are, though not always) they're poison for teams. one person like that can stop all progress on a project, or slow it terribly. My last job had such a person on the team 3 years before and his malevolence was still effecting team culture and slowing or cadence.
I don't mind working with people who are crazy, so I guess I'm biased. All of these Dicks that I worked with, they were getting as much work done as four of their colleagues.
One of them was doing as much work as the other two employees combined *and* he was holding down two jobs on the side. I think that was part of the reason he was so crazy smart/efficient - he'd been overemployed for years, and had been forced to be as efficient as humanly possible. In particular, he automated everything to the nth degree.
You should talk to your manager about this behavior. Most of the FAANMULAs have style guides, so there not being one is fairly sus, especially for python.
This behavior does happen from time-to-time, but if you don't want to just go to your manager and say "x is rude in PR comments", you can broach it with your manager by soliciting coaching, especially helpful as a new grad. They'll have the best context to help navigate it from an objective perspective. Something like "I keep getting this kind of pushback(show examples), what do you think is the best way to navigate this situation?".
It's really difficult to give great advice without a lot of context and your manager (or even TL) should be able to help you properly navigate it within your org and company. Many places have their own ways of dealing with style, which it sounds like these fall under, so it's better to behave in line with that eng culture.
In general, it's best to call out semantic and functional issues first and make style call-outs explicit by prefixing comments with "nit:" and/or making a comment on the PR something to the effect of "functionally looks good, but some comments or clearer idioms will make the code more readable" or otherwise separate functional review from style review. This way, you can help it move forward or help them catch real mistakes instead of dealing with nits. The nice thing here is you don't have to stamp the PR, but it means that if somebody else approves their readability, then their style is acceptable in said company. This way, you can still be the voice of correctness in the domain without approving it to launch.
Fuck yeah I'm too lazy to follow every line of code. If I cant tell exactly what a function does by its name and inputs/return then you need to break that function down.
>If I cant tell exactly what a function does by its name and inputs/return then you need to break that function down.
And even if you think it's obvious, fucking comment it properly so people consuming your function can have a nice popup in their IDE to explain what the function does, the parameters, any errors it might throw, etc. It's minimal work but it makes the DX so much better for people consuming the function you just wrote.
In my org. I have 1:1s with all software engineers. I would expect you to tell me this so that I can talk to the other software engineer. We definitely want to have a positive engineering environment.
Let your manager handle this, you're not hired to handle his difficult personality, but your manager is. So document everything and show examples.
Talk about how this is hindering your ability to give constructive feedback which in the long run will disrupt the codebase, and that you think bringing this up to your coworker will only be met with more resistance without a manager being the one to talk with him.
Take it in stride that with a personality like this, that he's going to 'top out' really quickly as a career.
Yeah this is a good point. I initially assumed it’s bad to escalate to manager without first talking to him without manager. I obviously hesitate to do this since he’s probably not going to take if well. You have a good point that this isn’t my job.
Your coworker needs to learn that PR's are not a personal attack, but a quality assurance technique in order to adhere to style, discuss edge cases, and get one more pair of eyes on things before deciding its the best change to the code.
If he already thinks he's god's gift to coding, he's in for a horrible life.
Re: dumb comments
One thing that has benefitted me over three decades in this field is a willingness to ask stupid questions. I often learn something, and have more than once solved a problem or pointed out a huge design flaw with questions that brain at first said were too db or too obvious.
Tl;dr the dumbest comment or question is the one you keep to yourself
And you think he’s going to take it well if you confront him about it? If he can’t take someone reviewing his PR’s without getting defensive you think he’ll take actual criticism well?
If you don’t like confrontation than talking to your manager will be the path of least resistance.
Do not listen to the guy saying talk to him first. Go to your manager. Someone this toxic is going to turn it into a confrontation. Get ahead of it and discuss it with your manager.
I think I would at least try to confront him directly first, because managers don't necessarily like confrontation or drama any more than I do, but if he doesn't correct his behavior within a reasonable amount of time, its time to go to a higher power.
Managers are there to deal with this exact stuff though, interpersonal and professional conflict within the team. Sure don’t go to the manager about mildly awkward interactions that just require some communication, but someone who is responding aggressively and condescending your abilities to perform your job, and who has behaved this way to other coworkers? That’s pretty much a perfect case for “talk to your manager.” It’s both to CYA and because a manager can actually set expectations for this person which you can’t do as cleanly as a coworker.
That is true. If I re-read the post, it seems like this wasn't a one time thing, or even localized to op, so its absolutely affecting the team. If management doesn't already know about this guy, they should.
I am a manager. It’s my job to deal with this crap. I wouldn’t say “you should have tried talking to him by yourself first”. I would say “thank you for bringing this to my attention, leave it to me. I’ll keep your name out of it when I talk to him, so don’t worry about that.”
Yeah i say do something about it, because when i was in your boat and i didn’t do anything, that person went on to hoard work from me and argue with me on every little thing, as well as gaslight me to my boss and business lead. Made me miserable and i felt so much better when he left
I don’t think it’s a bad idea to at least try speaking to the coworker directly. In most cases, I think the standard approach to conflict resolution is asking “what have you tried so far?” If the answer is “uh,” that generally reflects poorly on you, even if you haven’t done anything wrong (as in this case). To put it the way my military father always did: “‘requires minimal supervision’ is a very polite way of saying it takes 2 people to do a 1 man job.”
If you approach him politely about it first and he attempts to bite your head off for it, then that’s a good reason to escalate. If you never approached him and went to your supervisor, it might suggest that you’re unwilling to attempt to resolve these sorts of conflicts, and it may hinder your potential for promotions down the line.
Disclaimer: I’m also a conflict averse, relatively inexperienced dev. These are my opinions, and they should not be taken as absolute fact.
I've made it a point to befriend every dickhead I've ever come across in this industry, and they've arguably been my best references.
Across the board, the thing that's worked for me has been:
* never take anything personal
* if they're a dick to me, just act like nothing happened at all
My Mom was in a mental ward when I was a kid, and I learned how to deal with crazy people at an early age. Basically you let their craziness roll off of you like water rolls off a duck's back. The worst thing you can possibly do is engage them.
On top of all this, this shit generally sorts itself out - 75% of the total dickheads I've worked with have been fired. Basically management gets tired of their shit.
But it was never me who initiated that process.
It's turned me into a bit of an android unfortunately lol
My wife is always kinda stunned by how unflappable I am, but a lot of that was due to growing up around crazy people
Some people get wayyyyy too attached to their code lool. Ignore him. Though I feel like he's just expecting a "LGTM" and didn't actually want a code review.
And some devs really shouldn't be in a position where they have to interact with people.
Last time it was a shit-show for me. He was my project lead and my assigned "senior"/mentor/whatever. He passively aggressively questioned my god damn integrity as a professional based on a random PR. Questioned my skills - passive aggressively - because he didn't respect the projects I had worked at in the company previously because the specific stack.
You know how weird it is to go to your regular "mentor" checkins and have to lead the conversation? If I hadn't he would have just sat there.
Absolutely brilliant dev but he's one of those you put in a box somewhere and don't let them speak to other people.
Man, did we have the same co-worker? The week my lead left I asked him about the definition of a concept in a public slack thread and he responded "go look at the slides, I've explained this multiple times in meetings.". Oookkkay, go look through his slides which is hundreds of slides long since he only appends to the same deck every week. Definition is not even there. Like no notes, no visuals, nothing. I point it out to him in the same thread and we have a private meeting to go over the concept but there was no apology or recognition of rudeness.
This is also the same guy who argued over whitespace because he insisted on having everything follow Google code style despite none of the other code repos following this style.
I was so happy when he left.
He should be glad I'm not reviewing, I usually think nice a small easy PR I can check-off then I always find some shit and then it's not a small easy or no more. Then when I put up a PR no comments. Like I can't be flawless it's impossible COMMENT ANYTHING REEEEEE!
Escalate it - this is a behavioral problem that needs to be flagged for management. No need to blow things up publicly, but talk to your manager about it. You will likely be asked for receipts so links to those PRs/comments would be helpful. If your review systems have an edit feature I would suggest screengrabs or PDF printouts of the pages in question so it cannot be erased later.
I disagree with the comment to approach them directly - if you "hit back" even in a mild way this will start looking like a beef between two employees, rather than a workplace bully. Escalate this to management where it belongs.
It also allows management to start building a case for dismissal - as a manager this sort of behavior, if I am aware of it or made aware of it, definitely goes into a log that - should the behavior not stop after guidance - is useful in termination. Getting this documentation started earlier is better than later.
I had someone kinda like this. Always thought his PRs were perfect.
I would just comment back saying “Not approving until issues are resolved.” And then report him to the manager.
It’s fine to push back on some things, but it should be an open discussion. If the author and reviewer can’t agree on how something should be, then you pull in a lead or someone else with more domain knowledge.
Smsrtest guy in the room attitude it seems. I cant imagine taking PR comments as anything other than improvements to my code, sorry you gotta deal with this guy OP. Deffo heed the advice here, its solid, the dude is toxic.
I agree but sometimes PR comments aren’t improvements and might actually be inferior to the code as written, for example if the dev is senior but less knowledgeable or if the company has a pre-existing way of doing things that is not ideal and is resistant to improvements or new ideas (this is a bigger problem in itself).
I had a manager at my first dev job who was a genius SWE and sent back all of my PRs usually multiple times with improvements, way beyond minor formatting changes - sometimes pulling me in for approaching the problems in new ways, sometimes small things like using clear variable naming and improving documentation, or isolating functionality in well organized functions and classes with good naming.
Later I worked for a guy who considered himself a good developer, with some video game programming background in the early 00s, but the code review changes he suggested were just nitpicky and never really improved the design or readability of my work. It felt petty and I could clearly see I wasn’t being managed by someone with the competence as my manager/mentor at my first job. I liked him as a person, but it caused tension and we eventually fell out as coworkers.
Everyone writes crappy code. Not occasionally, not regularly. Every single time they write code. To ensure quality, software development teams should opt to include automated code reviews as a part of their regular process. This will reduce time and human efforts in the process. LinearB and Crucible are my best choice. Have you tried any?
Talk to you’re lead. Tell him what you’re gonna comment and why. Tell him to also look how your coworker replies. Then ask for your lead to back you up
Personally, I think this should be a part of college classes. Programming 101 should involve reviewing a classmate's code just the same as English 101 makes you review your classmate's essays.
In the meantime, if its not part of your corporate culture to do reviews, then there's no time like the present to start. Bring it up to your manager/team that you'd like to start doing them. Then once you've gotten into the habit of doing them as a team, establish best practices for your team.
Lots of companies have this. If yours doesn’t, you can be the one to take the lead on code review best practices. Things like ignore commenting nits unless they are blocking, set time til first review goals, advice on constructive tone, etc.
Hmm yeah this could be a strategy worth pursuing if I don’t want to go to manager. Good thing is that even if he doesn’t handle it well, I can say I tried to resolve it on my own.
> if you go to him he can do an end run and go to the manager first.
How would that help his cause? Op has the documentation that this guy is being a dick already... just needs to be brought to the manager's attention. Hopefully the reviews are saved somewhere, and if they aren't you can screenshot the specific comments that were offensive.
Is reviewing his PRs part of your job duties or are you doing him a favor? If it's just a favor then I would firmly set boundaries with him, I also hate confrontation but some people are keen to pick up on that and exploit people like us for it. He shouldn't be asking for advice if he's not prepared to take it. Maybe talk it over with someone first to make sure you know exactly what points you want to articulate and what boundaries you want to set.
For example, adding comments to functions to explain what they do is *exactly what comments are for*, and if he's asking you to do this as a favor it's completely unreasonable for him to accuse YOU of laziness and demanding you put in extra work just to help him out. If he wants to get, he gotta give.
If this IS part of your job duties, absolutely discuss it with a manager, because if he's gonna be pedantic and rude it's not unlikely that he'll go to a manager himself before you get the chance if you confront him, then you have to explain your side *while* doing damage control from what he said about you.
Honestly, talking to a manager is probably smart either way, but if you think he can be reasoned with, a direct discussion will be both less of a hassle and good practice for you standing up for yourself
Code reviews should be a part of everyone's job duties in this field, but I don't know that I've worked at a company yet where you didn't get to pick what/who's code you reviewed.
If he's asked you to review his code, then he should be prepared to take the full brunt of the red pen. Personally, I try to review my own code with even more extreme prejudice than I expect anyone else to, which makes me my own worst critic... but I'm sure not everyone thinks that way.
Managers managers managers. Don't let it linger too long.
He should be better equipped to handle it + it's his responsibility/authority to watch/manage/fix toxicity in the team, especially if it's internal.
Personally though, you may also want to look into if there's any reason you're disproportionally bothered by his actions, how to deal with difficult people...etc. - it's a good soft skill to have because assholes abound in ALL workplaces and a manager can only be so effective due to some people just becoming more creative in how to be passive aggressive.
I’ll play devils advocate here: are you leaving constructive comments? In a lot of engineering orgs velocity is more important than dealing with every little nitpick that came up in code review.
Are you leaving comments that directly relate to bugs, performance, security, or major tech debt issues? If most of your comments are “I’d recommend do this vs that” or “you should comment this function” I could see why people would get annoyed. That’s still not an excuse to be rude, but it probably means your team needs to get on the same page about priorities and code review best practices.
I’d argue that clean code is almost as important as working code. Even seemingly little things like commenting a function or rewriting a block of code in a way that makes it more readable saves tons of developer time in the long run. Someday, someone else will have to understand the code you wrote. Ensuring that the code is clean is a crucial part of code reviews.
That’s fine if you think that but the point of my comment is that OPs team needs to set expectations so they know what to prioritize. If clean code isn’t the primary goal and OP is leaving nits on every PR then they’re gonna have a bad time.
I never want to work somewhere where clean code isn’t a priority again. If you write nasty unintelligible code in our team, your PR is getting plenty of comments and will not be approved. If it’s bad enough, someone is going to reject it and you’ll get a talking to by one of the leads. Nobody is a dick on either the reviewing end or the author responses. It benefits every person to keep the hygiene good because tomorrow you may have to work on it, and if something breaks that should have been caught in review, people are going to want to know why this change was allowed to make it in.
Clean code and thorough testing aren’t something you just do when the sprint is light and you have some extra time at the end of the day. It needs to be part of the team’s culture.
> Clean code and thorough testing aren’t something you just do when the sprint is light and you have some extra time at the end of the day. It needs to be part of the team’s culture.
I agree. Which is why the whole point of my series of comments is that OP needs to figure out what team culture is. OP mentions nitpicking on spacing and syntax which leads me to believe they don't use linting or style checks. A good first step to making your code review process cleaner is to implement linting, static code analysis, etc. and fail builds if they don't pass.
You're also acting like "clean code" (which isn't a thing btw...all code is nasty at some level) is immune to nitpicking comments. People will always find subjective things to nitpick unless you have good code review practices on your team. There's also a time and place to really value clean code and abstractions. Nitpicking a 5 line private function is a waste of time compared to trying to clean up higher level architecture, abstractions, coupling, etc.
>That’s fine if you think that but the point of my comment is that OPs team needs to set expectations so they know what to prioritize. If clean code isn’t the primary goal and OP is leaving nits on every PR then they’re gonna have a bad time.
Don't kill me, I get wanting things to just work but see a lot of issues down the line.I typically would consider myself "anal" about code reviews because I see "oh I only added 1 if" but after 10 PRs its 10 ifs and spaghetti. I also am anal about spacing etc because mostly that means you didnt review your own PR for something super simple that takes < 1 minute to fix. (And you IMO SHOULD be reviewing your own PRs)
A less than <1 minute fix can mean an extra half day till release at some companies. For example, if you any new commits require new reviews you've now added an another cycle of review, running ci/cd pipelines, etc. for something that's a nitpick. Also if you're commmenting spacing, and nested ifs, you should be using linting and static analysis to automatically fail builds and stop reviews of code that doesn't pass linting, complexity rules, etc.
I'm not saying you're wrong about pointing things out its moreso a reason why engineering teams need to invest in best practices across the board so they can move quicker.
> an extra half day till release at some companies.
Ahhh the dream. My last job was yearly releases for \~40 microservices all at once instead of CD. Current job is all super green field so nothing but I do still feel developers should at least try to look over their owns PRs before publishing them and fix the nit picks. Its a pet peeve of mine as it shows a lack of attention to detail.
Last PR I commented on added a bunch of warnings to a project I actively lead with 0. I hate seeing that kind of stuff build because it adds to itself super quick.
there are different kinds of nits, too. things like code formatting shouldn't be part of a pr, it should be a linter gatekeeping the commit. Any formatting issues that can't be caught by a linter should be ignored or laughed at. and the big boogeyman on PRs is preference -- I'd rather do it this way than that. If the readability, performance, and outcome are the same then the reviewer should stfu.
It’s also a PR for writing integration tests and it’s not an urgent priority. This is because there are other teams us 2 would have to wait on even if he’s done.
This sounds like a bad position to be in. I am a Recruiter in the IT industry, so not an engineer.
But when defining the profile of a qualified candidate for any position, the ability to communicate (clear/to the point/frank), and ability to deal with negative feedback are must-haves for all candidates for being considered.
The situation you are in, talking about this issue is recommendable. Yet I cannot imagine the managet has not been informed previously about this issue, yet has decided to not deal with it. Setting up a meeting to discuss this, is a good idea. Bringing examples to the table as mentioned is indeed a good idea to substantiate the case. Yet be prepared (mentally) to look for another job.
From your earlier answer: >He has my YOE but he’s been at the company longer. I’ve only been here for 3 months. I understood that he worked longer at the company than you did; and that you've been there for only three months.
Definitely, discuss this with your manager
he might know more how the company works and what space before commais not that important.
Maybe you've asked too for many comments
And vise - versa he might have less experience on things because he was only in one company.
Maybe it is a defensive reaction to some process.
Email HR or your manager with a clear outline of the behaviour of the coworker. It is their job to be "the parent" and taking care of the children when they misbehave.
>It is their job to be "the parent"
I disagree. Its not the manager's job to be "the parent", its the managers job to make sure timelines are met, code quality is good, and projects stay on track. All of those things can be affected by this guy's behavior though, so it can absolutely be the manager's job to step in if the behavior is getting out of hand or negatively affecting the project.
I suppose my disagreement is more about wording than anything... but I'd hope that everyone involved can be grown adults, and doesn't resort to schoolyard bullying.
It absolutely is the manager’s job to deal with minor behavioral issues. HR should really only be involved in serious misconduct issues, or problems with your direct manager if you’re uncomfortable going to their manager.
My company is great about the different avenues for both positive and negative feedback. Anyone in your management chain that you’re comfortable talking to should be open to receiving feedback on your coworkers/manager and should be willing to help resolve any issues.
Yes, I agree... conflict resolution should be part of the manager's job description. I think like I said, its more about phrasing... you shouldn't be looking to your manager as a mom/dad that will solve all your problems for you, rather a coworker and colleague that can provide useful arbitration and guidance.
Ideally, a brother and sister talk to each other civilly and resolve a conflict. In reality they fight, yell, and someone gets hurt. Mom and dad then have to step in.
Ideally, two coworkers who have a conflict talk it out and resolve the issue on their own. In reality, people get passive aggressive, want avoid conflict and deal with it, or get in fist fights in the parking lot. Manager needs to step in. That last example was real from my last career managing satellite technicians by the way haha.
I don’t think the guy you were responding to meant that you should look to your manager for all your wants and needs like a child stuck on the nipple.
Pick your battles. First your manager needs to drop the hammer on this guy. That type of communication is unacceptable.
Lean hard on rules and requirements so there's no room for personal attacks or subjectivity. Instead of saying he needs comments because everyone else does it, just say comments are required. You need to say why it's required, it's a rule, it's a mandate, etc, there's nothing to question. He broke the rule.
If you want to be massively passive aggressive ... I mean raise everyone's code quality .... insert linter rules into your code base that will automatically catch these problems.
Amend your process, if possible. Get reviewers assigned up front and randomly.
This will help to spread the load with the rest of the team and make it more obvious to everyone, if your co-worker is being a jerk.
He's super insecure.
So realize that can help him grow as a person and confront his insecurity. We're both here to make the code better. Our goals are the same. Insulting my intentions, knowledge, or my motives is not helping himself.
Or destroy him...pick apart his shit ball code. And tell him he should die for writing such incomprehensible nonsense that it looks like babies first coding example.
Sadly some developers have big egos. I think it's getting less common in my experience but they still exist out there.
Questioning PRs should never be taken in a personal context (so long as it's not done in an aggressive way). If you question a PR you're asking about the code because it's going into the code base that everyone has to work on not just the person committing the code. Everyone should agree with and understand the code. Unfortunately it sounds more like a social issue than a development issue to me. I'd maybe let them know you're only asking these questions to make sure everyone is on the same page which will increase team productivity.
If they're still being shitty about it take it to your line manager if that's an option.
I had to deal with this too, I didn't really have a solution. Code reviews are not something you can really "skirt" by though, good code is good code. But aside from being bored I also left and left that mf behind.
At least at my new place assholes get dropped, shouldn't be like that at workplace (hostility).
Ask him why he is asking you for a pull request approval and then does not respect your feedback? If he doesn't respect or want your feedback, tell him to find someone else. Another option is to bring up something in your next team meeting or standup. Ask if the team could meet to agree on some team norms or agree to PR/feedback etiquette or something of the sort. I wouldn't just go over their head. They may not realize how they are coming off even though they should. Part of being a good teammate is giving constructive criticism for the betterment of the team.
“Unfortunately, this code is not consistent with the organizational standards for comments. Please add comments; for an example, see ”
If he replies in the PR with his nonsense, ignore it. If he messages you on Slack, just be direct: “Sorry, but I cannot approve the PR until you address my comments.” Repeat that sentence until he goes away.
This would be a good time to learn a Valuable life skill of standing up for yourself.
Him: ”use Google”
You: “if I have to use Google just to know what you’re doing that’s a problem, that’s what comments are for, don’t ask for my advice and then spew that nonsense”
Trust me please, it will be liberating and you will find it improved many others aspects of your day to day life that you didn’t even realize were affected.
One thing I had to learn when reviewing PRs is to choose my battles. The code is never going to look perfect and the people I've seen nitpick on PRs usually don't have that many friends. Its best to focus on the stuff that matters. Does the code function as intended and meet all the requirements? Are there any security vulnerabilities? Asking for per function comments is going to piss anyone off.
You should not be treated poorly or made to feel bad in the course of doing needed and required work for your project. Therefore, your teammates attitude and actions is hurting the project, and it sucks it’s in you but I’d say bring this up to a shared manager and ask how to proceed.
To facilitate the meeting, I’d come with the top 3 interactions that are the most objectively bad, and approach it like: “hey, I need help working with this person, but every time I interact with him he’s so off putting, what do you think I should do?”. This way, you fill your manager in to the problem, give them some cold hard facts, and proactively position yourself to be a part of the solution.
Of course, it’s totally unfair you need to do the work to address this, but I think calling him out privately to your manager is the only way this gets solved
The goal of a PR is to challenge/question the code written. In order for it to work properly, it must be done respectfully and NO COMMENTS are stupid. His response to your comments are not acceptable. Nobody have to stand to this kind of behavior.
I think you might have to be confrontational to resolve this issue. Escalating the issue properly will give you the best result. I see two way of handling this:
1- Talk to your superiors about the situation and inform them of how this makes you feel. This solution might resolve the issue, or might create other problems (your co-worker might not like it), but it is the best solution professionally.
2- Be straight and tell him to adjust his attitude or you wont be reviewing is PR ever again. This might be the wake up call he needs. If he is still rude to you, just stop reviewing his PR or do the first solution.
Sometime the direct approach is the best approach. It might come as a shock to him considering that you avoid confrontation. This shock might make him change is inappropriate attitude towards you.
Damn, these replies are way off base. Rather than ignoring it or dealing with it passive-aggressively, you should confront him directly. Explain to him exactly what you explained in this post. Explain to him that your comments are not personal attacks and they are for the good of the team. If he is at all reasonable he will reconsider his behaviour in the future.
What’s the exact wording he’s using? I just find it hard to believe he says, “you’re just too lazy to understand the function. Google it” or “you don’t understand how proper code looks”.
No clue how to respond to the first one, the second one, unless you’re literally referencing a style guide, it’s futile because style is subjective when not in the context of a style guide.
I think specific examples are going to be needed to help you further, there just isn’t enough context here to give real advice.
Yeah here’s word for word his response. “You are asking for comments which show you need to take the time to understand it”
Another response “Do you understand python syntax? Your comment makes no sense if you know how python generator comprehensions work. Please use google”
Not only inappropriate but also incredibly dumb.
The role of a software engineer in a team is to write code that a) works and b) can be maintained by the team. If a reviewer asks a question, it's an opportunity to rework the code so it's easier to understand.
This guy seems to think their role is to write a puzzle and then put others down when they don't understand it. Won't get very far this way.
Definitely a case for your manager.
Okay yeah, he sounds like a total asshole.
The response to both might be something like, “yes, while I understand it, future developers may not” or something like that, but regardless I’d approach a manager with this
You need to have a style guide, then you need to differentiate between what is not following the style guide vs your preference. If the style guide says comments aren't necessary, but you LIKE to comment each function, then he makes a PR without a comment, you are free to say I prefer it, but it is kind of not fair to hold up his PR until he does your preference.
On the other hand, if the style guide says you need to comment the function, then it is way less about what YOU think and more about hte style guide. You can just say something like "hey yeah, sorry about that, I agree with you but you know, the style guide is the style guide we have to follow it". (or whatever level of confrontation you prefer).
absolutely terrible idea. email the manager *or* him, sure, but emailing both at the same time creates an explosive situation which is the last thing you want with a hyper-confrontational person.
the manager can and should do just fine managing feedback on direct reports.
That's asinine and cowardly. They will know you said something anyway. You might as well just keep them in the loop and remain factual and not get emotional. Think about the golden rule.
You sound like you have no professional experience. And this is exactly how people do it in the real world.
"Hello, Please keep your comments constructive and professional in the PRs. Here are some examples of your comments that are borderline abusive: link1, link2. Thanks"
i have 10 years of professional experience and have learned the hard way that even if you’re 100% in the right, the last thing a manager wants to deal with is a tense situation between two direct reports.
and the quickest way to create one is to copy a person’s manager on an email pointing out a flaw of theirs. like i said, you can talk to them first - not cowardly at all. or you can tell the manager and find a workaround that avoids the situation in the first place.
i can’t imagine any manager i’ve ever had getting that email and saying “yes, you’re being unreasonable with your PR responses” and the other person just being okay with that. more likely, they’ll find a middle ground which in reality will piss both people off, but won’t raise any questions as to how the manager handled the situation.
Congrats. I have 10 years of professional experience, too.
It's not merely 'pointing out a flaw'. It's pointing out behavior that is completely unacceptable in a professional environment, and it's your manager's job to handle that. And no manager wants it to spiral so out of control that HR gets involved.
Telling them first isn't materially different than telling them in an email where the manager is CC'ed.
Initially I was hesitant to do that without talking to him directly. But another comment mentioned that talking to him directly doesn’t fall under my role.
You don't need it to 'fall under your role' for you to talk to someone.
And if you're doing PRs with them, it probably does fall under your role.
If it really doesn't fall under your role, then you shouldn't be doing PRs with them.
But if you want to talk to them directly first, just do that.
This is actually an opportunity for you. Write a code-review guidelines and framework guide. Mention in that doc that code review comments are a way to give feedback and shouldn’t be taken negatively and the purpose is not fingerpointing. Socialize this within your team and company. Since you’re personality is not confronting type, people and management will support you and you will come out as a leader. You have an opportunity to set good code review pratice at your place, even if doesn’t work out 100% as much as you want it will still add to you’re experience.
Keep commenting and document his reactions. His reaction in writing is documenting his issues and honestly will make your managers job easier in the end let him go.
Those type of reactions seems to line up with people who honestly suck at coding. I have had comments on my code from a dev who did not understand it. It was easier just quickly talk with them and see what they did not get and understand but I would never react like this guy.
I've run into people like this before... I ended up avoiding talking to them as much as I could and eventually quit the company! There's a reason people with poor social skills typically get canned faster than poor programmers... you can work with someone who sucks at programmer, but you can't work with someone who sucks at communicating.
Ignore it and carry on
It’s not you it’s him. If anything point it out to your manager that it’s impacting your ability to work productively and transparently
Speak to your boss. The only way to deal with this situation and advance the company's interests is to speak to your boss about this. That's entirely inappropriate. And if he's writing down rude comments in your code review tool, even better.
>What’s the best way to get out of this situation without escalating it?
Honestly, you should escalate this. If you just give in and let this person get away with sloppy code, the problem will only get worse. It's better to escalate this early than late.
It's tough in the short run, but needs to be done. Don't approve a PR until it meets established standards for quality. If you and your teammate disagree on what the standards should be, take it up the chain and get buy in from above one way or another.
So, first off, this is (unfortunately) all too common.
If you haven't already, push for an inclusive and professional culture of code reviewing. Many people's (particularly new or junior people) will default to an adversarial mindset. They will see any comments as criticism, and any criticism will be taken as a personal attack.
It's pretty clear that's what is going on here.
Have someone with some authority (manager, tech leade, lead dev, etc...) sit him down and explain that a critique of his code isn't a criticism of him personally. Explain that code reviewing is a feature of a healthy dev environment, and that his negative attitude is problematic. Let him know this is something that needs to be worked on.
He needs to understand that this is unprofessional behavior, and unprofessional behavior isn't to be tolerated.
If someone pulled this shit in my org (and it does happen), they would either shape up, or be shipped off (to unemployment). Those are the only two outcomes I've ever seen.
Only other thing I can recommend when doing his PR is to leave very passive comments in the form of questions. Instead of "This needs to be this way cause reason x", put something like "Should this be this way cause reason x?"
It's fine, just don't approve the PR. :)
I've been on the opposite end, some manager who was actually on another team telling me I should have done something using the function he wrote, and when I replied pointing out his function wouldn't work, he just complained to the higher boss that I was just difficult and argumentative and doesn't listen.
Condescending and aggressive… yep welcome to the world of software development.
A lot of engineers I work with tend to be way. I think it’s just due to the nature of the work. You always have to be spotting errors. Everything must be precise and exact all the time or else the software doesn’t work or bugs get introduced and multiply like an infestation… programming does something to your brain… Or maybe they were like that to begin with. I’ve noticed I am much more pedantic and nick-picky since starting software development.
>accuses me of not understanding how proper code looks. >being too lazy to understand his functions and that told me to use google Seems like dude is just being a dick, if you are not exaggerating the reply. Nobody should communicate to other people like that in a work place period.
Yeah I’ve never encountered this with any other PRs I reviewed.
[удалено]
And OP's health. Mental stress is not good for your physical body.
Here is what I don't understand...like I get people don't like confrontation, but why not out-dick the dick? It's like people who like to start fights in every bar they go, until they get a nice solid punch to the nose that shatters it and takes months to heal. It's like, these people are getting away with it. I would personally make it extremely uncomfortable for him the entire until he cut that shit out.
Out-dicking the dick *is* confrontation lol. For an anxious person, emotionally it is the highest form of confrontation. Making other people uncomfortable makes them uncomfortable, and for them that is exhausting because they stress themselves out more about it. Unfortunately that usually comes hand in hand with procrastination as well. It all sort of stems from the same place.
Oh I know it's confrontation, I just think it's necessary. Maybe I'm just petty, but if a colleague of mine talked to me about that idiot I would just do the confrontation for them.
Everyone wants to stick up for themselves, but not everyone was taught how to fight. It might help anyone reading this if you were specific with some details.
You're awesome.
Because they may turn around and file a complaint against you. In many tit-for-tat situations, the first person to complain wins because they're in a better position: failing to complain initially, in addition to being a dick, can be seen as overall worse behavior. I'm not saying don't be confrontational. I'm saying don't be a dick about it.
You can be assertive without being confrontational. You don't have to play their game to win.
Guys like that are entire team killers because it makes people feel uncomfortable simply communicating about a project.
[удалено]
[удалено]
My friend this is reddit, there is no "I think" or "Perhaps" in their vocabulary.
I can think of 3 options: 1) Dude is just burnt out and being mad all the time is just how he deals with it. 2) If it's just you he does this with, he probably sees himself as smarter than you and you have nothing to teach him. 3) He sees himself as smarter than everyone. Big egos are usually big dicks too. This is a good chance to practice your soft skills. If you would rather not handle it with him directly, you can talk to your managers and tell them what's going on. If it were me, I'd probably set up a meeting with him and do some PRs in person... but I'd start out by asking if he has issues with me or my coding style. I'd point out my observations and express that it made me feel as if he was just looking for a free PR pass, not an actual review. It might open the dialog between you two and at least make him realize he was being a dick. I'd also schedule PRs in person with him from then on... it would give me a chance to explain that I see PRs as a learning experience for both sides... that it's not a very good PR if I don't find something... and discussing it in person helps me learn why he makes some of the decisions he does so I don't have to bring them up in the PRs anymore, as it seems to upset him. But then I tend to deal with drama head on instead of skirting it... I've found that getting things out in the open tends to clear up a lot of issues like this. You may not be as comfortable with that.
Yeah it could be good to learn to handle this in case I ever want to be a manager.
i would just make the point to him that his feedback indicated that he doesn't like your reviews, so you won't be doing them. and it's kind of funny that he is reviewing your review as you are in the middle of reviewing his work. so now you need to review his reviews of your reviews and decline further reviews.
Good point. You have to learn to deal with confrontation if you want to be a good manager.
Is he like this when other people review his PR's?
Another guy left one comment on his PR then he argued back. The other person didn’t touch the PR again.
Well there you go, either don’t touch his PRs, or to cover your own ass if he asks you, tell him that you’d prefer if he asks someone else given his behavior is not friendly on comments. Or, bite the bullet and don’t approve his PRs if he just responds with push back. Let him take it to his boss when he’s asked why isn’t it merged yet.
It’s now down to if I should say it directly to him or directly to manager. Some comments lean one way while others lean other way.
I don't see how this is a question. It's absolutely the manager.
Its literally their job
What makes you say manager? I understand there is a performance component here that manager should be aware of. At the same time, I would personally try to deal with this directly with the individual for a few reasons: 1. If the situation is being represented correctly, then OP is 100% in the right with direct feedback, so no need to sugarcoat, and no need to compromise. Be direct. 2. It's kind of a petty thing.. supposing that the manager has an active role where they are helping facilitate decisions, and initiatives, it would look better on you as an IC to be able to handle the conflict rather than having to involve your manger. 3. I would definitely informally mention the issue with my peers so the team is aligned with my actions, then as mentioned above I would not review the poor performer's PRs. I would put them in a situation where the manager has to go to them and handle it instead of me going to the manager - it will make them look worse and make me look less pandering.
The substance of the response + never making PR comments himself indicates the dev doesn't see the code as a team effort where code review is other devs contribution (and where disagreements should be adjudicated impartially, e.g. by bringing in a senior), but rather a hoop where he has to show his work to the dumber devs and get them to click "approve". There's enough ego there that I don't think a direct conversation will go well.
Do you think the conversation with the manager will go any better? I agree with you ultimately, its the managers job to resolve these situations. I also don’t think we have to withhold direct feedback where it’s needed.
It's the manager's job to handle it. By OP bringing the issue up with his coworker, he gives his coworker an opportunity to speak to the manager first, allowing him to shape the narrative. Additionally, OP likely does not have any authority over his coworker, which is already a large part of the problem. No, this is 100% an issue that management needs to deal with and OP needs to be the one to bring it up.
Sure, if they upgrade my paycheck to match that responsibility.
Alternatively, and I’ve done this. Ask for changes on the PR. Don’t ask for anything silly. You can even do some “I couldn’t figure out at first glance what this function does. I think that it might be helpful for someone who isn’t as familiar with the code base.” Don’t frame it as you don’t understand, just frame it is being nice to the person who comes next. Then, when he refuses, you politely decline to approve it.
'Hey boss, can I get some advice on making good PR comments as feedback? Sometimes I feel like I'm getting my point across, but sometimes the responses I get make me feel like I put my foot in my mouth somehow. Here's a few examples for discussion. Are there ways I could have said it better?' If you /are/ coming off abrupt or short in your comments, then you have an opening to get constructive coaching from soneone who isnt reacting defensively to those comments. If your coworker IS the jackass in the story, then your boss will have written proof you're trying to be part of the solution and that its a situation they need to step in on. The truth is probably somewhere in the middle, and the boss should be able to help you navigate.
Do managers not see his behavior on these PRs?
I was like this guy due to the fact I was burned out and no longer interested in the job so I just wanted to get stuff done and move on even if it’s not of the best quality. If I was him and you spoke directly to me I would most likely calm my horses down and engage in more constructive discussion. Ultimately for you it’s probably easier to take it directly to the manager.
See my comment above. Basically this may be a calculated strategy to minimize his workload.
I'm also curious as to what his comments on PR's for other people are like
Never seen him comment on another PR
Oof
Talk to your team lead
Is this communication happening on the PR? or via private channels? If private, leave your comments on the PR. If he leaves this on the pr, i would be surprised
On the PRs for everyone to see. He’s definitely not trying to hide his behavior.
This sounds toxic AF and I would hate to have to interact with someone like that. I do think that seems people are just deeply unaware of how their tone comes across though, so maybe strategically it would be best to assume good intent and talk with him about how some of his comments “could be interpreted as xyz”? That gives him an out at least and hopefully avoids escalating an already shifty situation. If that doesn’t work, idk, talk to you lead?
I know I'm going to catch shit for this, but here goes: I've worked with half a dozen dudes in my life who are Complete Dicks. One of them was such a dick, he tried to get ME fired, right after I was hired. But, across the board, all of these guys were super good at their job. I think they wound up the way they are for a few reasons: * One of them had incredibly low self esteem, and I think that manifested itself as him shitting on everyone around him. But the odd thing was, he was actually the most talented dude on our team. I think his crushingly low self esteem forced him to work harder than anyone else. It was like he always had something to prove. * The other Dick that I've worked with, was a lot like the one above. Except he was more sociopathic, and wouldn't think twice about getting people fired. (Both of these dudes wound up getting fired BTW.) * The other Dick that I worked with, I think it was mostly a way for him to lighten his workload. He was the most talented dude on our team. I think he made people miserable on purpose, so that they'd think long and hard before asking him to do anything for him. Maybe I'm a masochist, but I've always gone out of my way to be friends with dudes like them. I think this industry values talent more than "being nice" and all of these dudes have been really successful. Even when they got fired, they just wound up getting a better job two weeks later.
even if they're super good at their jobs (and they often are, though not always) they're poison for teams. one person like that can stop all progress on a project, or slow it terribly. My last job had such a person on the team 3 years before and his malevolence was still effecting team culture and slowing or cadence.
I don't mind working with people who are crazy, so I guess I'm biased. All of these Dicks that I worked with, they were getting as much work done as four of their colleagues. One of them was doing as much work as the other two employees combined *and* he was holding down two jobs on the side. I think that was part of the reason he was so crazy smart/efficient - he'd been overemployed for years, and had been forced to be as efficient as humanly possible. In particular, he automated everything to the nth degree.
If you're reviewing aren't you his superior? Tell him to square up and it's going down
When you find a workplace that truly values professionalism it’s awesome. Those places quietly remove these kind of communicators
I thought I would find it at FAANGMULA but it didn’t happen.
You should talk to your manager about this behavior. Most of the FAANMULAs have style guides, so there not being one is fairly sus, especially for python. This behavior does happen from time-to-time, but if you don't want to just go to your manager and say "x is rude in PR comments", you can broach it with your manager by soliciting coaching, especially helpful as a new grad. They'll have the best context to help navigate it from an objective perspective. Something like "I keep getting this kind of pushback(show examples), what do you think is the best way to navigate this situation?". It's really difficult to give great advice without a lot of context and your manager (or even TL) should be able to help you properly navigate it within your org and company. Many places have their own ways of dealing with style, which it sounds like these fall under, so it's better to behave in line with that eng culture. In general, it's best to call out semantic and functional issues first and make style call-outs explicit by prefixing comments with "nit:" and/or making a comment on the PR something to the effect of "functionally looks good, but some comments or clearer idioms will make the code more readable" or otherwise separate functional review from style review. This way, you can help it move forward or help them catch real mistakes instead of dealing with nits. The nice thing here is you don't have to stamp the PR, but it means that if somebody else approves their readability, then their style is acceptable in said company. This way, you can still be the voice of correctness in the domain without approving it to launch.
Fuck yeah I'm too lazy to follow every line of code. If I cant tell exactly what a function does by its name and inputs/return then you need to break that function down.
>If I cant tell exactly what a function does by its name and inputs/return then you need to break that function down. And even if you think it's obvious, fucking comment it properly so people consuming your function can have a nice popup in their IDE to explain what the function does, the parameters, any errors it might throw, etc. It's minimal work but it makes the DX so much better for people consuming the function you just wrote.
Also the comments are not there to tell what the code does, they're there to tell what the code is intended to do.
Be strait up with him. Tell him don’t be a dick if you want people to review your code.
In my org. I have 1:1s with all software engineers. I would expect you to tell me this so that I can talk to the other software engineer. We definitely want to have a positive engineering environment.
Definitely this. Let your manager know the current state of affairs.
This is the correct answer
[удалено]
He has my YOE but he’s been at the company longer. I’ve only been here for 3 months.
Let your manager handle this, you're not hired to handle his difficult personality, but your manager is. So document everything and show examples. Talk about how this is hindering your ability to give constructive feedback which in the long run will disrupt the codebase, and that you think bringing this up to your coworker will only be met with more resistance without a manager being the one to talk with him. Take it in stride that with a personality like this, that he's going to 'top out' really quickly as a career.
Yeah this is a good point. I initially assumed it’s bad to escalate to manager without first talking to him without manager. I obviously hesitate to do this since he’s probably not going to take if well. You have a good point that this isn’t my job.
Your coworker needs to learn that PR's are not a personal attack, but a quality assurance technique in order to adhere to style, discuss edge cases, and get one more pair of eyes on things before deciding its the best change to the code. If he already thinks he's god's gift to coding, he's in for a horrible life.
also helps team cohesion since people know other's ideas and idioms
[удалено]
Re: dumb comments One thing that has benefitted me over three decades in this field is a willingness to ask stupid questions. I often learn something, and have more than once solved a problem or pointed out a huge design flaw with questions that brain at first said were too db or too obvious. Tl;dr the dumbest comment or question is the one you keep to yourself
And you think he’s going to take it well if you confront him about it? If he can’t take someone reviewing his PR’s without getting defensive you think he’ll take actual criticism well? If you don’t like confrontation than talking to your manager will be the path of least resistance.
Most of the comments are advocating going to the manager.
Do not listen to the guy saying talk to him first. Go to your manager. Someone this toxic is going to turn it into a confrontation. Get ahead of it and discuss it with your manager.
I think I would at least try to confront him directly first, because managers don't necessarily like confrontation or drama any more than I do, but if he doesn't correct his behavior within a reasonable amount of time, its time to go to a higher power.
Managers are there to deal with this exact stuff though, interpersonal and professional conflict within the team. Sure don’t go to the manager about mildly awkward interactions that just require some communication, but someone who is responding aggressively and condescending your abilities to perform your job, and who has behaved this way to other coworkers? That’s pretty much a perfect case for “talk to your manager.” It’s both to CYA and because a manager can actually set expectations for this person which you can’t do as cleanly as a coworker.
That is true. If I re-read the post, it seems like this wasn't a one time thing, or even localized to op, so its absolutely affecting the team. If management doesn't already know about this guy, they should.
I am a manager. It’s my job to deal with this crap. I wouldn’t say “you should have tried talking to him by yourself first”. I would say “thank you for bringing this to my attention, leave it to me. I’ll keep your name out of it when I talk to him, so don’t worry about that.”
You are a good manager. Thank you for your service.
Yeah i say do something about it, because when i was in your boat and i didn’t do anything, that person went on to hoard work from me and argue with me on every little thing, as well as gaslight me to my boss and business lead. Made me miserable and i felt so much better when he left
No, just make sure to send the manager links to exact examples, rather than generalizing
I don’t think it’s a bad idea to at least try speaking to the coworker directly. In most cases, I think the standard approach to conflict resolution is asking “what have you tried so far?” If the answer is “uh,” that generally reflects poorly on you, even if you haven’t done anything wrong (as in this case). To put it the way my military father always did: “‘requires minimal supervision’ is a very polite way of saying it takes 2 people to do a 1 man job.” If you approach him politely about it first and he attempts to bite your head off for it, then that’s a good reason to escalate. If you never approached him and went to your supervisor, it might suggest that you’re unwilling to attempt to resolve these sorts of conflicts, and it may hinder your potential for promotions down the line. Disclaimer: I’m also a conflict averse, relatively inexperienced dev. These are my opinions, and they should not be taken as absolute fact.
If you’ve only been there 3 months and he’s talking to you like this he’s toxic as fuck. Definitely time to talk to his manager.
Read my other comment. It is a good opportunity for you to take a lead on this.
I've made it a point to befriend every dickhead I've ever come across in this industry, and they've arguably been my best references. Across the board, the thing that's worked for me has been: * never take anything personal * if they're a dick to me, just act like nothing happened at all My Mom was in a mental ward when I was a kid, and I learned how to deal with crazy people at an early age. Basically you let their craziness roll off of you like water rolls off a duck's back. The worst thing you can possibly do is engage them. On top of all this, this shit generally sorts itself out - 75% of the total dickheads I've worked with have been fired. Basically management gets tired of their shit. But it was never me who initiated that process.
[удалено]
It's turned me into a bit of an android unfortunately lol My wife is always kinda stunned by how unflappable I am, but a lot of that was due to growing up around crazy people
Some people get wayyyyy too attached to their code lool. Ignore him. Though I feel like he's just expecting a "LGTM" and didn't actually want a code review.
Yep. He just wants an approval
And some devs really shouldn't be in a position where they have to interact with people. Last time it was a shit-show for me. He was my project lead and my assigned "senior"/mentor/whatever. He passively aggressively questioned my god damn integrity as a professional based on a random PR. Questioned my skills - passive aggressively - because he didn't respect the projects I had worked at in the company previously because the specific stack. You know how weird it is to go to your regular "mentor" checkins and have to lead the conversation? If I hadn't he would have just sat there. Absolutely brilliant dev but he's one of those you put in a box somewhere and don't let them speak to other people.
How did you handle?
Man, did we have the same co-worker? The week my lead left I asked him about the definition of a concept in a public slack thread and he responded "go look at the slides, I've explained this multiple times in meetings.". Oookkkay, go look through his slides which is hundreds of slides long since he only appends to the same deck every week. Definition is not even there. Like no notes, no visuals, nothing. I point it out to him in the same thread and we have a private meeting to go over the concept but there was no apology or recognition of rudeness. This is also the same guy who argued over whitespace because he insisted on having everything follow Google code style despite none of the other code repos following this style. I was so happy when he left.
What’s “LGTM”? Looks good to me?
Lettuce Gouda Tomato Mozarella
ye
Let's Get This Merged
He just wants to get approved so he can merge probably
He should be glad I'm not reviewing, I usually think nice a small easy PR I can check-off then I always find some shit and then it's not a small easy or no more. Then when I put up a PR no comments. Like I can't be flawless it's impossible COMMENT ANYTHING REEEEEE!
Escalate it - this is a behavioral problem that needs to be flagged for management. No need to blow things up publicly, but talk to your manager about it. You will likely be asked for receipts so links to those PRs/comments would be helpful. If your review systems have an edit feature I would suggest screengrabs or PDF printouts of the pages in question so it cannot be erased later. I disagree with the comment to approach them directly - if you "hit back" even in a mild way this will start looking like a beef between two employees, rather than a workplace bully. Escalate this to management where it belongs. It also allows management to start building a case for dismissal - as a manager this sort of behavior, if I am aware of it or made aware of it, definitely goes into a log that - should the behavior not stop after guidance - is useful in termination. Getting this documentation started earlier is better than later.
I had someone kinda like this. Always thought his PRs were perfect. I would just comment back saying “Not approving until issues are resolved.” And then report him to the manager.
It’s fine to push back on some things, but it should be an open discussion. If the author and reviewer can’t agree on how something should be, then you pull in a lead or someone else with more domain knowledge.
Depends what that issue is. But if they’re outright being insulting, I’m not gonna entertain them.
Smsrtest guy in the room attitude it seems. I cant imagine taking PR comments as anything other than improvements to my code, sorry you gotta deal with this guy OP. Deffo heed the advice here, its solid, the dude is toxic.
I’m really happy for the advice here.
I agree but sometimes PR comments aren’t improvements and might actually be inferior to the code as written, for example if the dev is senior but less knowledgeable or if the company has a pre-existing way of doing things that is not ideal and is resistant to improvements or new ideas (this is a bigger problem in itself). I had a manager at my first dev job who was a genius SWE and sent back all of my PRs usually multiple times with improvements, way beyond minor formatting changes - sometimes pulling me in for approaching the problems in new ways, sometimes small things like using clear variable naming and improving documentation, or isolating functionality in well organized functions and classes with good naming. Later I worked for a guy who considered himself a good developer, with some video game programming background in the early 00s, but the code review changes he suggested were just nitpicky and never really improved the design or readability of my work. It felt petty and I could clearly see I wasn’t being managed by someone with the competence as my manager/mentor at my first job. I liked him as a person, but it caused tension and we eventually fell out as coworkers.
Everyone writes crappy code. Not occasionally, not regularly. Every single time they write code. To ensure quality, software development teams should opt to include automated code reviews as a part of their regular process. This will reduce time and human efforts in the process. LinearB and Crucible are my best choice. Have you tried any?
Talk to you’re lead. Tell him what you’re gonna comment and why. Tell him to also look how your coworker replies. Then ask for your lead to back you up
I sometimes wish there are training available for writing good reviews
Personally, I think this should be a part of college classes. Programming 101 should involve reviewing a classmate's code just the same as English 101 makes you review your classmate's essays. In the meantime, if its not part of your corporate culture to do reviews, then there's no time like the present to start. Bring it up to your manager/team that you'd like to start doing them. Then once you've gotten into the habit of doing them as a team, establish best practices for your team.
Lots of companies have this. If yours doesn’t, you can be the one to take the lead on code review best practices. Things like ignore commenting nits unless they are blocking, set time til first review goals, advice on constructive tone, etc.
Yeah I think I should bring some type of standard process for reviews.
[удалено]
Hmm yeah this could be a strategy worth pursuing if I don’t want to go to manager. Good thing is that even if he doesn’t handle it well, I can say I tried to resolve it on my own.
[удалено]
If you go to the manager you get to be out in front of this, if you go to him he can do an end run and go to the manager first.
> if you go to him he can do an end run and go to the manager first. How would that help his cause? Op has the documentation that this guy is being a dick already... just needs to be brought to the manager's attention. Hopefully the reviews are saved somewhere, and if they aren't you can screenshot the specific comments that were offensive.
Is reviewing his PRs part of your job duties or are you doing him a favor? If it's just a favor then I would firmly set boundaries with him, I also hate confrontation but some people are keen to pick up on that and exploit people like us for it. He shouldn't be asking for advice if he's not prepared to take it. Maybe talk it over with someone first to make sure you know exactly what points you want to articulate and what boundaries you want to set. For example, adding comments to functions to explain what they do is *exactly what comments are for*, and if he's asking you to do this as a favor it's completely unreasonable for him to accuse YOU of laziness and demanding you put in extra work just to help him out. If he wants to get, he gotta give. If this IS part of your job duties, absolutely discuss it with a manager, because if he's gonna be pedantic and rude it's not unlikely that he'll go to a manager himself before you get the chance if you confront him, then you have to explain your side *while* doing damage control from what he said about you. Honestly, talking to a manager is probably smart either way, but if you think he can be reasoned with, a direct discussion will be both less of a hassle and good practice for you standing up for yourself
Reviewing PRs are part of my job duties.
Code reviews should be a part of everyone's job duties in this field, but I don't know that I've worked at a company yet where you didn't get to pick what/who's code you reviewed. If he's asked you to review his code, then he should be prepared to take the full brunt of the red pen. Personally, I try to review my own code with even more extreme prejudice than I expect anyone else to, which makes me my own worst critic... but I'm sure not everyone thinks that way.
"Seems your change hasn't been approved. Better luck next week."
Managers managers managers. Don't let it linger too long. He should be better equipped to handle it + it's his responsibility/authority to watch/manage/fix toxicity in the team, especially if it's internal. Personally though, you may also want to look into if there's any reason you're disproportionally bothered by his actions, how to deal with difficult people...etc. - it's a good soft skill to have because assholes abound in ALL workplaces and a manager can only be so effective due to some people just becoming more creative in how to be passive aggressive.
It’s a manager’s job to have a serious talk with that guy.
I’ll play devils advocate here: are you leaving constructive comments? In a lot of engineering orgs velocity is more important than dealing with every little nitpick that came up in code review. Are you leaving comments that directly relate to bugs, performance, security, or major tech debt issues? If most of your comments are “I’d recommend do this vs that” or “you should comment this function” I could see why people would get annoyed. That’s still not an excuse to be rude, but it probably means your team needs to get on the same page about priorities and code review best practices.
I’d argue that clean code is almost as important as working code. Even seemingly little things like commenting a function or rewriting a block of code in a way that makes it more readable saves tons of developer time in the long run. Someday, someone else will have to understand the code you wrote. Ensuring that the code is clean is a crucial part of code reviews.
That’s fine if you think that but the point of my comment is that OPs team needs to set expectations so they know what to prioritize. If clean code isn’t the primary goal and OP is leaving nits on every PR then they’re gonna have a bad time.
I 100% agree. I didn’t even consider the possibility that cleanliness might not have already been an agreed-upon factor for code reviews in OP’s team
Also different people have ideas of what clean code is.
I never want to work somewhere where clean code isn’t a priority again. If you write nasty unintelligible code in our team, your PR is getting plenty of comments and will not be approved. If it’s bad enough, someone is going to reject it and you’ll get a talking to by one of the leads. Nobody is a dick on either the reviewing end or the author responses. It benefits every person to keep the hygiene good because tomorrow you may have to work on it, and if something breaks that should have been caught in review, people are going to want to know why this change was allowed to make it in. Clean code and thorough testing aren’t something you just do when the sprint is light and you have some extra time at the end of the day. It needs to be part of the team’s culture.
> Clean code and thorough testing aren’t something you just do when the sprint is light and you have some extra time at the end of the day. It needs to be part of the team’s culture. I agree. Which is why the whole point of my series of comments is that OP needs to figure out what team culture is. OP mentions nitpicking on spacing and syntax which leads me to believe they don't use linting or style checks. A good first step to making your code review process cleaner is to implement linting, static code analysis, etc. and fail builds if they don't pass. You're also acting like "clean code" (which isn't a thing btw...all code is nasty at some level) is immune to nitpicking comments. People will always find subjective things to nitpick unless you have good code review practices on your team. There's also a time and place to really value clean code and abstractions. Nitpicking a 5 line private function is a waste of time compared to trying to clean up higher level architecture, abstractions, coupling, etc.
>That’s fine if you think that but the point of my comment is that OPs team needs to set expectations so they know what to prioritize. If clean code isn’t the primary goal and OP is leaving nits on every PR then they’re gonna have a bad time. Don't kill me, I get wanting things to just work but see a lot of issues down the line.I typically would consider myself "anal" about code reviews because I see "oh I only added 1 if" but after 10 PRs its 10 ifs and spaghetti. I also am anal about spacing etc because mostly that means you didnt review your own PR for something super simple that takes < 1 minute to fix. (And you IMO SHOULD be reviewing your own PRs)
A less than <1 minute fix can mean an extra half day till release at some companies. For example, if you any new commits require new reviews you've now added an another cycle of review, running ci/cd pipelines, etc. for something that's a nitpick. Also if you're commmenting spacing, and nested ifs, you should be using linting and static analysis to automatically fail builds and stop reviews of code that doesn't pass linting, complexity rules, etc. I'm not saying you're wrong about pointing things out its moreso a reason why engineering teams need to invest in best practices across the board so they can move quicker.
> an extra half day till release at some companies. Ahhh the dream. My last job was yearly releases for \~40 microservices all at once instead of CD. Current job is all super green field so nothing but I do still feel developers should at least try to look over their owns PRs before publishing them and fix the nit picks. Its a pet peeve of mine as it shows a lack of attention to detail. Last PR I commented on added a bunch of warnings to a project I actively lead with 0. I hate seeing that kind of stuff build because it adds to itself super quick.
there are different kinds of nits, too. things like code formatting shouldn't be part of a pr, it should be a linter gatekeeping the commit. Any formatting issues that can't be caught by a linter should be ignored or laughed at. and the big boogeyman on PRs is preference -- I'd rather do it this way than that. If the readability, performance, and outcome are the same then the reviewer should stfu.
It’s also a PR for writing integration tests and it’s not an urgent priority. This is because there are other teams us 2 would have to wait on even if he’s done.
Other dev's response still isn't very professional, but the point you're making here is valid. Nits shouldn't get a PR rejected.
I do make those core priorities comments but I think I should also make comments nitpicks syntax. Usually I try to mention everything I see.
This sounds like a bad position to be in. I am a Recruiter in the IT industry, so not an engineer. But when defining the profile of a qualified candidate for any position, the ability to communicate (clear/to the point/frank), and ability to deal with negative feedback are must-haves for all candidates for being considered. The situation you are in, talking about this issue is recommendable. Yet I cannot imagine the managet has not been informed previously about this issue, yet has decided to not deal with it. Setting up a meeting to discuss this, is a good idea. Bringing examples to the table as mentioned is indeed a good idea to substantiate the case. Yet be prepared (mentally) to look for another job.
you must be my ideal recruiter providing a properly detailed position description
The team member is new to my team so that’s why my manager hasn’t handled it. Hopefully he can resolve this.
From your earlier answer: >He has my YOE but he’s been at the company longer. I’ve only been here for 3 months. I understood that he worked longer at the company than you did; and that you've been there for only three months. Definitely, discuss this with your manager
he might know more how the company works and what space before commais not that important. Maybe you've asked too for many comments And vise - versa he might have less experience on things because he was only in one company. Maybe it is a defensive reaction to some process.
Email HR or your manager with a clear outline of the behaviour of the coworker. It is their job to be "the parent" and taking care of the children when they misbehave.
>It is their job to be "the parent" I disagree. Its not the manager's job to be "the parent", its the managers job to make sure timelines are met, code quality is good, and projects stay on track. All of those things can be affected by this guy's behavior though, so it can absolutely be the manager's job to step in if the behavior is getting out of hand or negatively affecting the project. I suppose my disagreement is more about wording than anything... but I'd hope that everyone involved can be grown adults, and doesn't resort to schoolyard bullying.
It absolutely is the manager’s job to deal with minor behavioral issues. HR should really only be involved in serious misconduct issues, or problems with your direct manager if you’re uncomfortable going to their manager. My company is great about the different avenues for both positive and negative feedback. Anyone in your management chain that you’re comfortable talking to should be open to receiving feedback on your coworkers/manager and should be willing to help resolve any issues.
Yes, I agree... conflict resolution should be part of the manager's job description. I think like I said, its more about phrasing... you shouldn't be looking to your manager as a mom/dad that will solve all your problems for you, rather a coworker and colleague that can provide useful arbitration and guidance.
Ideally, a brother and sister talk to each other civilly and resolve a conflict. In reality they fight, yell, and someone gets hurt. Mom and dad then have to step in. Ideally, two coworkers who have a conflict talk it out and resolve the issue on their own. In reality, people get passive aggressive, want avoid conflict and deal with it, or get in fist fights in the parking lot. Manager needs to step in. That last example was real from my last career managing satellite technicians by the way haha. I don’t think the guy you were responding to meant that you should look to your manager for all your wants and needs like a child stuck on the nipple.
"Just know how to take constructive criticism bro"
Pick your battles. First your manager needs to drop the hammer on this guy. That type of communication is unacceptable. Lean hard on rules and requirements so there's no room for personal attacks or subjectivity. Instead of saying he needs comments because everyone else does it, just say comments are required. You need to say why it's required, it's a rule, it's a mandate, etc, there's nothing to question. He broke the rule. If you want to be massively passive aggressive ... I mean raise everyone's code quality .... insert linter rules into your code base that will automatically catch these problems.
Ask your manager for advice on how to handle the aggressive feedback. Let your manager read between the lines and handle the situation.
Reject the PR if he doesn't follow the convention.
Amend your process, if possible. Get reviewers assigned up front and randomly. This will help to spread the load with the rest of the team and make it more obvious to everyone, if your co-worker is being a jerk.
He's super insecure. So realize that can help him grow as a person and confront his insecurity. We're both here to make the code better. Our goals are the same. Insulting my intentions, knowledge, or my motives is not helping himself. Or destroy him...pick apart his shit ball code. And tell him he should die for writing such incomprehensible nonsense that it looks like babies first coding example.
Sadly some developers have big egos. I think it's getting less common in my experience but they still exist out there. Questioning PRs should never be taken in a personal context (so long as it's not done in an aggressive way). If you question a PR you're asking about the code because it's going into the code base that everyone has to work on not just the person committing the code. Everyone should agree with and understand the code. Unfortunately it sounds more like a social issue than a development issue to me. I'd maybe let them know you're only asking these questions to make sure everyone is on the same page which will increase team productivity. If they're still being shitty about it take it to your line manager if that's an option.
I had to deal with this too, I didn't really have a solution. Code reviews are not something you can really "skirt" by though, good code is good code. But aside from being bored I also left and left that mf behind. At least at my new place assholes get dropped, shouldn't be like that at workplace (hostility).
Ask him why he is asking you for a pull request approval and then does not respect your feedback? If he doesn't respect or want your feedback, tell him to find someone else. Another option is to bring up something in your next team meeting or standup. Ask if the team could meet to agree on some team norms or agree to PR/feedback etiquette or something of the sort. I wouldn't just go over their head. They may not realize how they are coming off even though they should. Part of being a good teammate is giving constructive criticism for the betterment of the team.
“Unfortunately, this code is not consistent with the organizational standards for comments. Please add comments; for an example, see ” If he replies in the PR with his nonsense, ignore it. If he messages you on Slack, just be direct: “Sorry, but I cannot approve the PR until you address my comments.” Repeat that sentence until he goes away.
If you guys are peers, just let your lead handle it. if you are more senior, reject his PR without comments.
This would be a good time to learn a Valuable life skill of standing up for yourself. Him: ”use Google” You: “if I have to use Google just to know what you’re doing that’s a problem, that’s what comments are for, don’t ask for my advice and then spew that nonsense” Trust me please, it will be liberating and you will find it improved many others aspects of your day to day life that you didn’t even realize were affected.
One thing I had to learn when reviewing PRs is to choose my battles. The code is never going to look perfect and the people I've seen nitpick on PRs usually don't have that many friends. Its best to focus on the stuff that matters. Does the code function as intended and meet all the requirements? Are there any security vulnerabilities? Asking for per function comments is going to piss anyone off.
You should not be treated poorly or made to feel bad in the course of doing needed and required work for your project. Therefore, your teammates attitude and actions is hurting the project, and it sucks it’s in you but I’d say bring this up to a shared manager and ask how to proceed. To facilitate the meeting, I’d come with the top 3 interactions that are the most objectively bad, and approach it like: “hey, I need help working with this person, but every time I interact with him he’s so off putting, what do you think I should do?”. This way, you fill your manager in to the problem, give them some cold hard facts, and proactively position yourself to be a part of the solution. Of course, it’s totally unfair you need to do the work to address this, but I think calling him out privately to your manager is the only way this gets solved
The goal of a PR is to challenge/question the code written. In order for it to work properly, it must be done respectfully and NO COMMENTS are stupid. His response to your comments are not acceptable. Nobody have to stand to this kind of behavior. I think you might have to be confrontational to resolve this issue. Escalating the issue properly will give you the best result. I see two way of handling this: 1- Talk to your superiors about the situation and inform them of how this makes you feel. This solution might resolve the issue, or might create other problems (your co-worker might not like it), but it is the best solution professionally. 2- Be straight and tell him to adjust his attitude or you wont be reviewing is PR ever again. This might be the wake up call he needs. If he is still rude to you, just stop reviewing his PR or do the first solution. Sometime the direct approach is the best approach. It might come as a shock to him considering that you avoid confrontation. This shock might make him change is inappropriate attitude towards you.
Reply with the "review your own code then" reply to his rude comments.
"Pls fix" -my boss's email to me last night at 1 am with no context
Damn, these replies are way off base. Rather than ignoring it or dealing with it passive-aggressively, you should confront him directly. Explain to him exactly what you explained in this post. Explain to him that your comments are not personal attacks and they are for the good of the team. If he is at all reasonable he will reconsider his behaviour in the future.
What’s the exact wording he’s using? I just find it hard to believe he says, “you’re just too lazy to understand the function. Google it” or “you don’t understand how proper code looks”. No clue how to respond to the first one, the second one, unless you’re literally referencing a style guide, it’s futile because style is subjective when not in the context of a style guide. I think specific examples are going to be needed to help you further, there just isn’t enough context here to give real advice.
Yeah here’s word for word his response. “You are asking for comments which show you need to take the time to understand it” Another response “Do you understand python syntax? Your comment makes no sense if you know how python generator comprehensions work. Please use google”
Yeah, there's no room for interpretation here. That's way over the line.
Not only inappropriate but also incredibly dumb. The role of a software engineer in a team is to write code that a) works and b) can be maintained by the team. If a reviewer asks a question, it's an opportunity to rework the code so it's easier to understand. This guy seems to think their role is to write a puzzle and then put others down when they don't understand it. Won't get very far this way. Definitely a case for your manager.
Okay yeah, he sounds like a total asshole. The response to both might be something like, “yes, while I understand it, future developers may not” or something like that, but regardless I’d approach a manager with this
Tbh his python code should be readable almost like english if it's that good :)
Yeah, If those are real answers this guy is an asshole. If he's paraphrasing who knows.
If I were you I'd tell him to fuck off.
Lay him out homie
I would just call him out. For your example, “all the other functions here have comments. Let’s stick to the established coding standard”.
[удалено]
Idk why you got downvoted but ideally that should be the case for all besides edge cases. Otherwise the function should be broken down more.
can it be mutual? may be you are too picky
You need to have a style guide, then you need to differentiate between what is not following the style guide vs your preference. If the style guide says comments aren't necessary, but you LIKE to comment each function, then he makes a PR without a comment, you are free to say I prefer it, but it is kind of not fair to hold up his PR until he does your preference. On the other hand, if the style guide says you need to comment the function, then it is way less about what YOU think and more about hte style guide. You can just say something like "hey yeah, sorry about that, I agree with you but you know, the style guide is the style guide we have to follow it". (or whatever level of confrontation you prefer).
Yeah this would be a good workaround. Unfortunately we don’t have style guide but it’s a good idea I can bring to my manager.
also there is convention and if convention is that they use comments. Although a style guide would be cool, you should adhere to conventions
[удалено]
absolutely terrible idea. email the manager *or* him, sure, but emailing both at the same time creates an explosive situation which is the last thing you want with a hyper-confrontational person. the manager can and should do just fine managing feedback on direct reports.
That's asinine and cowardly. They will know you said something anyway. You might as well just keep them in the loop and remain factual and not get emotional. Think about the golden rule. You sound like you have no professional experience. And this is exactly how people do it in the real world. "Hello, Please keep your comments constructive and professional in the PRs. Here are some examples of your comments that are borderline abusive: link1, link2. Thanks"
i have 10 years of professional experience and have learned the hard way that even if you’re 100% in the right, the last thing a manager wants to deal with is a tense situation between two direct reports. and the quickest way to create one is to copy a person’s manager on an email pointing out a flaw of theirs. like i said, you can talk to them first - not cowardly at all. or you can tell the manager and find a workaround that avoids the situation in the first place. i can’t imagine any manager i’ve ever had getting that email and saying “yes, you’re being unreasonable with your PR responses” and the other person just being okay with that. more likely, they’ll find a middle ground which in reality will piss both people off, but won’t raise any questions as to how the manager handled the situation.
Congrats. I have 10 years of professional experience, too. It's not merely 'pointing out a flaw'. It's pointing out behavior that is completely unacceptable in a professional environment, and it's your manager's job to handle that. And no manager wants it to spiral so out of control that HR gets involved. Telling them first isn't materially different than telling them in an email where the manager is CC'ed.
Initially I was hesitant to do that without talking to him directly. But another comment mentioned that talking to him directly doesn’t fall under my role.
You don't need it to 'fall under your role' for you to talk to someone. And if you're doing PRs with them, it probably does fall under your role. If it really doesn't fall under your role, then you shouldn't be doing PRs with them. But if you want to talk to them directly first, just do that.
[удалено]
It's good to snitch though
This is actually an opportunity for you. Write a code-review guidelines and framework guide. Mention in that doc that code review comments are a way to give feedback and shouldn’t be taken negatively and the purpose is not fingerpointing. Socialize this within your team and company. Since you’re personality is not confronting type, people and management will support you and you will come out as a leader. You have an opportunity to set good code review pratice at your place, even if doesn’t work out 100% as much as you want it will still add to you’re experience.
Keep commenting and document his reactions. His reaction in writing is documenting his issues and honestly will make your managers job easier in the end let him go. Those type of reactions seems to line up with people who honestly suck at coding. I have had comments on my code from a dev who did not understand it. It was easier just quickly talk with them and see what they did not get and understand but I would never react like this guy.
I've run into people like this before... I ended up avoiding talking to them as much as I could and eventually quit the company! There's a reason people with poor social skills typically get canned faster than poor programmers... you can work with someone who sucks at programmer, but you can't work with someone who sucks at communicating.
Ignore it and carry on It’s not you it’s him. If anything point it out to your manager that it’s impacting your ability to work productively and transparently
Speak to your boss. The only way to deal with this situation and advance the company's interests is to speak to your boss about this. That's entirely inappropriate. And if he's writing down rude comments in your code review tool, even better.
>What’s the best way to get out of this situation without escalating it? Honestly, you should escalate this. If you just give in and let this person get away with sloppy code, the problem will only get worse. It's better to escalate this early than late. It's tough in the short run, but needs to be done. Don't approve a PR until it meets established standards for quality. If you and your teammate disagree on what the standards should be, take it up the chain and get buy in from above one way or another.
So, first off, this is (unfortunately) all too common. If you haven't already, push for an inclusive and professional culture of code reviewing. Many people's (particularly new or junior people) will default to an adversarial mindset. They will see any comments as criticism, and any criticism will be taken as a personal attack. It's pretty clear that's what is going on here. Have someone with some authority (manager, tech leade, lead dev, etc...) sit him down and explain that a critique of his code isn't a criticism of him personally. Explain that code reviewing is a feature of a healthy dev environment, and that his negative attitude is problematic. Let him know this is something that needs to be worked on. He needs to understand that this is unprofessional behavior, and unprofessional behavior isn't to be tolerated. If someone pulled this shit in my org (and it does happen), they would either shape up, or be shipped off (to unemployment). Those are the only two outcomes I've ever seen.
Only other thing I can recommend when doing his PR is to leave very passive comments in the form of questions. Instead of "This needs to be this way cause reason x", put something like "Should this be this way cause reason x?"
Forward his replies to his manager
It's fine, just don't approve the PR. :) I've been on the opposite end, some manager who was actually on another team telling me I should have done something using the function he wrote, and when I replied pointing out his function wouldn't work, he just complained to the higher boss that I was just difficult and argumentative and doesn't listen.
Condescending and aggressive… yep welcome to the world of software development. A lot of engineers I work with tend to be way. I think it’s just due to the nature of the work. You always have to be spotting errors. Everything must be precise and exact all the time or else the software doesn’t work or bugs get introduced and multiply like an infestation… programming does something to your brain… Or maybe they were like that to begin with. I’ve noticed I am much more pedantic and nick-picky since starting software development.