How to deal with an*l engineers?
Posted by New_Firefighter1683@reddit | ExperiencedDevs | View on Reddit | 110 comments
I'm 14 YOE and I've never dealt with something like this.
I've worked on several large projects, led several, etc. So I have a pretty good idea what I'm doing.
I recently got moved to a new team and it's... not pleasant. The engs on this team are SUPER anal about their preferences on everything.
I get good advice here and there on PRs. Things that are confusing, things i should add comments for, super welcome.
There are code quality things too, like refactoring other parts I didn't really work on or are relevant to my story, but those are welcome too in certain situations.
But there are times when I have hard deadlines on a big feature. With only a week to build out something pretty large, I was rushing to get things done. However, things are unit tested, things work, code is as clean as I could make it given the time constraints, etc. But on this PR, I got 104(!) PR comments.
Renaming variables, breaking out functions into separate modules for reuse, breaking out structs used in private classes to modules, etc. And super big changes like redoing the entire queue system which I had ALREADY aligned on the team with, but everyone changed their minds....
Now I'm blocked since people aren't approving the PR. And it's supposed to be out tomorrow and no way that's going to happen.
This isn't isolated either. It happens on like 75% of my PRs. People having full on philosophical conversations on my PRs. Even a simple 2 liner bugfix ends up turning into refactoring 3 different classes thrown into it.
I'm not meeting deadlines because of this. On top of that, priorities change willy nilly even though I align with 5 different people on it. A > B > C. I'm halfway through A and suddenly "we need B NOW". I context switch to B, spend half a day on it and suddenly "external client needs C now and it's urgent". So I jump on C, and now A is hitting a hard deadline. Meanwhile, on all 3 of the features, I have about 800 PR comments for changes.
I document everything, but everyone wants 5 page essays with complete diagrams and flowcharts for everything, and my PRs are blocked until I get those in. Something basic would end up taking 2 full days and I'm only allotted about half a day on it.
I communicate HEAVILY and transparently, but it gets me nowhere.
I have no idea how to deal with this. Everything is a slog.
DownRampSyndrome@reddit
> "This isn't isolated either. It happens on like 75% of my PRs. People having full on philosophical conversations on my PRs. Even a simple 2 liner bugfix ends up turning into refactoring 3 different classes thrown into it."
This recommended change is outside the scope of work and hasnt been accounted for in our estimations. Please create a new task for this and add it to the backlog so that velocity can be recorded accurately. Or some bullshit like that.
fedsmoker9@reddit
Yup this is the way. “It’s not within the scope of this ticket”
muuchthrows@reddit
I find that this doesn’t really work, the obvious response is:
”So writing good code is not within the scope of this ticket?”
There’s a misunderstanding somewhere around what is considered good enough code quality for the product or project.
pausethelogic@reddit
I think that response would only come from someone more junior. Writing “good” code is rarely ever the goal, the goal is always to provide business value
Imoa@reddit
“The code here is tested and meets requirements for an MVP for the story. Please make a new story for refactoring and improving the code - it’s outside the scope of this story”
gajop@reddit
The ticket rarely ever talks about code quality, and testing is only one such aspect. If the PR breaks coding conventions (explicit or implicit by the fact there is existing code), involves bad practices for the framework or language, is poorly architectured, too tightly coupled, "unreadable"/convoluted by whatever subjective metric the reviewer has, then sometimes it might make sense to fix before merging - especially if the reviewer is more senior and knows what they're talking.
As a general rule, you shouldn't worsen the quality of code already there. And you should make an effort to improve the quality continuously. Developers who just dish features with subpar code quality and create a maintenance nightmare are hard to work with. Someone else needs to run around fixing it, and that only happens once really big issues start happening. PMs almost never make room for refactoring/code improvement as it doesn't have immediate apparent value, so you need to do it continuously as you work if you want it done - within every sprint imo.
You probably shouldn't give 100 comments the day before a deadline (if it's a real deadline). That's just not considerate. Or if you give comments, you should separate truly blocking ones that need to be addressed now, and those that should be done immediately after the deadline.
Imoa@reddit
I agree that writing good code is important - and making sure that code fits conventions is arguably an implicit requirement of all code (if not outright explicit). If the code fits convention though, meets requirements, is tested / includes tests, and isn’t atrociously written / structured, then at some point the teammates feedback / refusal to approve a PR becomes a blocker and needs to be discussed with his manager.
It’s pretty binary - the code either meets the team’s minimum requirements or it doesn’t. Any quality feedback above that line is just nice-to-have, and would need justification to delay a deadline.
gajop@reddit
I think the problem lies in determining minimum requirements. There is and always will be a certain amount of subjective opinion on it. No linter or AI tool can cover everything, especially not the more complex aspects of architecture/code structure.
Involving management is fine, but it won't work out for you if you are junior/new to the team and it's your lead that's giving you excessive feedback. You're likely to just end up adding some process to it (e.g. mandatory design / mock reviews to reduce the chance of "big" asks at final code review stage).
OP has 12 YOE though so that might work out differently for him. He should definitely talk with engineers as well, in a public channel too. Maybe do an in-person review too, as people are much less likely to be nitpicky when next to you.
Imoa@reddit
It’s definitely a YMMV situation but I think as a general rule of thumb, if the feedback is asking for entire rewrites and refactors of large blocks of code like OP is experiencing then the person requesting that needs to justify it. If the rewrite would delay a delivery then it needs to have a solid justification probably beyond just quality unless it is performance critical code
Even a senior reviewing a junior’s code should be able to justify such a large request and shouldn’t be frequent. It’s the sort of thing that should be pretty easy to bring to management even for a junior if someone is making frivolous requests for large rewrites - as or you say, that person should be able to explain their reasoning 1:1, and you can hear them out before bringing anything up with management.
For better or worse most teams I’ve been on were very delivery oriented, quality was a nice to have from the developers but not a need to have. It resulted in some traffic jams from poor quality occasionally but it makes navigating the type of situation OP is in extremely easy - the leads and management would have a much more critical eye on an amount or type of feedback that was causing delays.
woeful_cabbage@reddit
Such a boring way to live
teslas_love_pigeon@reddit
While I share the sentiment, the type of coworker OP is describing does not work in good faith. They just want to extract value out of people, they don't care about the product or profession. They just want control.
Imoa@reddit
I agree to an extent but not if it’s causing delays that impact deadlines. If it’s fixable within reason and not drastically changing the size of the story.
OPs team sounds like it’s giving feedback that drastically changes the size of tasks - that goes beyond “just fix it” territory.
DownRampSyndrome@reddit
"Your concerns around the quality of the code should be had with the original author of the module. The scope of the work for this task is to deliver business value. Conversations around changes that don't add business-value would be best moved to the refactoring ticket once you create it."
Teh_Original@reddit
Part of the problem with the original author part is people grow and change, so something that looked good to them a long time ago may look bad now.
DownRampSyndrome@reddit
The point of the response isn't to actually have them reach out to the original author and provide feedback (no doubt that piece of code has probably been touched by many hands at this point) - it's to politely tell them to fuck off because they're wasting time and effort focusing on the wrong thing.
Putting the burden of creating the issue etc back in their court can be an effective way to guide them to discovering whether or not the work is worth the effort, or if they just think its worth the effort because they're not the ones doing it.
It wouldn't be the first time someone has come to the conclusion that if its not worth writing an issue up to do the work, then the work itself isn't worth doing. If they feel enough conviction that the work is worth doing, then they can own that ticket.
SkullLeader@reddit
Cleaning up bad code unrelated to this ticket is not in the scope of this ticket. Certainly not if I don't need to touch it for this ticket. And if I only need to make a minor change to it for this ticket, while cleaning it up would certainly be nice, if time does not permit it should be acceptable not to. After all I am not making anything worse than it already is by not cleaning it up. I'm pretty sure that when the OP's story was sized, cleaning up all code that he may end up touching to implement the story was not considered part of the effort.
It really sounds like there is some huge inconsistencies with how code is reviewed in the OP's team. Like how did such code make it past the PR's in which it was added if its truly that bad? Why is it only being flagged now? it sounds like some of the people commenting on these PR's are seeing the code only for the first time.
vancity-@reddit
Correct. Writing working code is within the scope of this ticket.
Jira ticket
CRAP-432 - Clean up foois already scheduled to address the issues raised. Also want to revisit thefooBarFactoryas it's a bit janky right now. Need a couple days but @deadbeef needs these changes out asap.... At least that's how I'd respond.
New_Firefighter1683@reddit (OP)
Tried this. "Well, clean code shouldn't be an afterthought, I'm not approving until this is changed"
Middle-Comparison607@reddit
Yes, and the sooner you stop resisting the better for you
vinny_twoshoes@reddit
This happened to me once and I tried so hard to approach it professionally by proactively making a ticket to resolve their issue. And the response was some immature "I don't see what the big deal is, just fix it now", making me look like the lazy one. It was a tough position to be in because I was the newbie on the team.
sobe86@reddit
> I'm not meeting deadlines because of this.
As soon as something threatens a deadline you communicate that to your manager. You should say to your manager that this is hurting your productivity and send them examples.
> Even a simple 2 liner bugfix ends up turning into refactoring 3 different classes thrown into it.
You have to say no more, or even 'yes probably a good idea, we should add a ticket for it, beyond the scope of this PR though' - these people are dumping more unplanned work on you - is that part of their job description? Because otherwise you've got to say no.
New_Firefighter1683@reddit (OP)
This is coming from my manager. PM fights him but gets nowhere.
I would go to skip, but he's never around
SmellyButtHammer@reddit
Do you have the deadline conversations with your manager?
sobe86@reddit
OK well it's a huge red flag if your manager is doing this and doesn't care that this is causing you to miss deadlines. I don't want to jump to conclusions without full details, but this person sounds completely incompetent and you might consider trying to get away from this person. How did you end up on this team, do you have a route out of it?
GandolfMagicFruits@reddit
If the PM and the EM are fighting, you're going nowhere fast
WebMaxF0x@reddit
Some people here are really willing to die on their pointless hill.
First, check if it's a real deadline or some bullshit corporate agile deadline.
If it's a real deadline, get your manager or product owner up to speed. In good faith, tell them everything they need to make an informed decision. Where could you cut corners? What are the risks? What are the tradeoffs? Let them make the call and share the responsibility, it's their job. You now have a mandate to do whatever they decided which overrides your teammates and yourself.
If it's a fake deadline it depends. If you think the PR comments are worth it, cheap and safe do it now or in a follow-up PR. Worth it but expensive/risky/complex, split it off into a new ticket, it's now in the hands of your manager or product owner. Not worth it, accept that you may be wrong and in good faith ask clarifying questions until both you and the reviewer reach a common conclusion, or ask a third-party/coin-flip to decide.
Former_Dark_4793@reddit
Looks like anal isn’t your thing, stick with bushes
New_Firefighter1683@reddit (OP)
I can always tell which comments are unemployed
Former_Dark_4793@reddit
lol chill man
LebaneseLurker@reddit
Slap a TODO on there and tell them you’ll deal with it later but a deadline is a deadline and it works functionally
cballowe@reddit
Are you the only one running into these problems or is it everybody? Is this engineer on your team or is it an adjacent team where you occasionally need to make changes in code that they own?
I've been in positions where I had to be the anal engineer, but generally tried to avoid it. In practice, I had responsibility for a core system that many other teams had to interact with. There were certain code paths with terrible isolation that were frequently changed. The worst of these had years of "but it's just a 2 line fix" piled in and the teams making the changes weren't really on the hook for identifying and fixing it when something went wrong in another part of the system.
We required a basic writeup of what was changing, some acknowledgement that other interactions had been looked at, some basic statement of resource impacts, etc. These usually went really fast if someone reached out before they started, took our advice on how to accomplish their goal, etc. Where they ended up being painful was someone making the change without talking to us, not having answers to the basic questions, and then not taking "here's a better way to do that that will make it better for the next person too" advice.
There should be some sort of "how to make changes to this component" documentation somewhere, also things like style guides etc, and even engineering standards to follow. They should be able to point you at them. Usually the first line of those standards of "new code should follow the practices of existing code" but they should still be able to point you to the standards, and you should follow them. If they can't, and there's no engineering reason for their arguments, push back. Don't assume they're just being difficult, though.
tiethy@reddit
How many PRs did you make for this feature? Based on what you said, it sounds like you captured the entire feature in one PR.
If that’s the case, 100+ comments kind of makes sense and what you should do is break them up into smaller PRs so those comments can be federated into easier-to-manage PRs.
New_Firefighter1683@reddit (OP)
It should have been 4 separate PRs.
The problem is, in the past, it takes 1-2 days to get people to review it, even when I hound them, and this needed to go out ASAP, so I stuck it all into 1 PR.
However, it's really not that big. It's 2 classes. It's about 30 functions or so. It's really not that bad.
tiethy@reddit
Okay, based on the rest of your comments, what I'm gathering:
Everybody is subject to the behaviour of receiving +++ PR comments on their PRs so this isn't isolated to you.
Multiple people on your team stay until 9pm and leave 100+ PR comments on each others' diffs.
Despite aligning the team on technical approaches (with notes) prior to building things, the EM/other developers literally change their mind after you've already built things.
Your EM is reviewing code. Your EM is leaving 30+ comments on your PRs (probably related to changing what you've already aligned on). Your EM is not budging on deadlines despite being the one to break alignment and leaving all of those PRs.
Unfortunately... the problem of overly excessive (and I'll go ahead and assume counter-productive) PR reviews sounds like it starts from the top (your EM) and has spread its way to the rest of the team.
Some problems can be solved and some cannot. This is a problem that cannot be solved by you- it's one that will have to be solved by your EM.
If I were in your shoes, I'd just comply. Fix every trivial comment (like name changes). Respond to every non-trivial comment with "fixing this will take X amount of time, will be ready by (now + X) date." If your EM wants the changes to be done and is aware of the (now + X) date, your EM will be the one who decides whether the code is fixed or the deadline is hit.
Your job isn't to lead your EM- it's the other way around. If your EM tells you to do Y, then do Y and let your EM know how much time it will take. Your EM decides the priority of clean code vs. deadline. If your EM wants both, they'll have to find someone who will work +++ hours to do both.
Brief-Translator1370@reddit
Involve whoever is giving you that deadline when it happens. Let them know that these things are valid points but not necessary to block the PR
New_Firefighter1683@reddit (OP)
The person giving the deadline is the one changing their minds on priorities
midasgoldentouch@reddit
👀 Is this person a tech lead? EM? PM? Who’s the stakeholder(s) of a project? Is the deadline internal or external?
New_Firefighter1683@reddit (OP)
Great question :)
EM. PM jumps in once in a while to fight them. EM gets aggressive. PM gives up. EM changes their mind.
Western_Objective209@reddit
Your EM doesn't make any sense; they gives a short deadline and then blocks the PR, they can't seriously be upset if it doesn't meet the deadline. Sounds like a guy with ADHD who is overworked, which tbh is pretty common for an EM
New_Firefighter1683@reddit (OP)
Pretty much lol
blaine-exe@reddit
I think the solution is to not put yourself in a position to burn out. You have already seen that you can put in two engineers worth of effort and still be asked for more by someone who can't see that they are in their own way.
This is a really hard skill to learn in the software space. Put in your 6-9 hours a day, and don't do more. Don't engage in your EM's rage bait, and don't do crunch time on a weekly or even monthly basis.
My strategy (as suggested by others) is to politely explain that a review comment is valid but addressing it would affect the delivery timeline. Keep a list of items for later follow-up. If they insist, then the delivery is delayed, and you have proof of why when you point back to the conversation.
In my work, I have also found it helpful to create a design document for features more complicated than a dozen or two lines of code. A design doc includes inputs and output, plus important functional details, and it gets approved before starting code work. It helps both developers and reviewers, because each side can look to see what spec they are aligning toward. It reduces cognitive load on both sides.
As a developer specifically, it becomes simple to reply to pedantic philosophical arguments with a statement about the current code being to spec with the approved design. To change from spec after design approval would require revisiting the design document, also affecting the delivery date.
Godspeed, friend 🫡
slashedback@reddit
This EM sounds like a cocaine addict executive. Seriously, I’ve worked with a number of CEOs/CTOs/VPs of engineering that acted just like this.
krolyat@reddit
Do you not explain changing priority will change the deadline?
New_Firefighter1683@reddit (OP)
Yep. Response is "well I don't know what to tell you, both needs to get done"
usone32@reddit
There's always option c = keep an eye out for a new job!
jokestan@reddit
Let them deal with the problem then, “I can address this feedback but it’ll affect the delivery by X days, are you cool with that?”
Plourdy@reddit
Revise many of their requests to be ‘nice to have’ rather than ‘required’, or discuss the option of avoiding preemptive refactoring until needed. (IE don’t pull out code for reuse until you have a reason to reuse it)
ThlintoRatscar@reddit
First off, why is your PR so large that there can be 100+ comments on it?
You need to break that down into smaller increments.
Second, if you get 100+ comments, your PR is wildly inappropriate. That's a lot of effort by multiple people pointing out 100+ different issues.
Jamming it through by getting a manager to override the team because you've already invested significant time says that you can't engineer. This is the very definition of "sunk cost fallacy".
These are essentially bug reports, and if you have 100+ bugs on a single PR, you need to take a beat and completely reevaluate what you've done.
Thirdly, your colleagues are trying to help you out and make it good. You need to check your ego at the door, listen, and learn how to play nice in the sandbox.
Finally, with 100+ comments, you've wasted a lot of everyone's time. You need to apologize and make it right.
New_Firefighter1683@reddit (OP)
lol the specific PR I'm referring to is about 2 classes consuming a queue.
On any of the 4 companies I've been at across 7 different teams, it would not have garnered 100+ comments.
ThlintoRatscar@reddit
Sure. And at this one, it did.
Why do you suppose that is?
It could be that they're a bunch of anal retentive jerks.
Or, it could be because your code sucks.
One of those protects your ego, while the other requires work.
If you truly think they're just anal, quit and prove them wrong by doing something impressive.
If not, check your ego, take their advice, and fix your code.
woeful_cabbage@reddit
Gottem
mrsunshine2012@reddit
Idk, that’s a lot of assumptions into a pretty vague scenario. If you have two chatty seniors who are happy to duplicate comments (I.e. “rename this variable” every time the variable is referenced) you can hit 100 pretty quick.
I’ve seen both sides of this dynamic - sloppy author who’s trying to rush code into main, as well as anal reviewer who nitpicks everything to a crawl. Both are bad, very hard to say who’s in the right based on a one sided post.
ThlintoRatscar@reddit
https://xkcd.com/1513/
pugworthy@reddit
If they are all like that then it’s you, not them. You need to work to their standards.
Evinceo@reddit
Demanding that they fix something outside the scope of the PR in order to approve the PR which is causing slipped deadlines isn't good reviewer behavior.
pugworthy@reddit
No it’s terrible behavior. I never said it was good.
If ONE developer was doing all this then it’s a problem that can be fixed. If it’s the whole team, it’s time to leave.
Cokemax1@reddit
then you should have said. "leave the team". rather than you sound like anomaly.
pugworthy@reddit
Anomaly is something that stands out, which doesn’t mean good or bad.
New_Firefighter1683@reddit (OP)
Nope.
2 other engineers on my team quietly said to me "I don't see the issue TBH, it looks fine to me, but I'm going to defer to the EM (who put 30+ comments"
Fabulous-Arrival-834@reddit
This is a wild assumption. Office politics is real and OP can genuinely be getting blocked for no reason. Given the amount of performance based layoffs that are happening, its totally common for teammates to turn on each other and make sure someone is performing worse than them so that they get saved from the next round of layoffs.
pugworthy@reddit
Yea and if that’s the case OP needs to get the hell out of there.
What I’m saying is that OP is the outlier in the group. OP is not going to change a think unless they have backup.
And if the group is supported by management and management likes it, then good luck. If management doesn’t care then again, good luck.
doctaO@reddit
Heard on a podcast recently that “the camel is a horse designed by committee.” Selectively not adhering to groupthink is one way to become a high performer. This sounds like a good situation for that.
bigdogsrus@reddit
Ask them to pair program with you and go painfully slow during asking lots of questions. When they try to leave your desk say I really would like to learn from you please don’t leave yet. Prevent them from getting their work done on time. Do this every single time. Build their ego with being so nice but also firm that they need to sit with you , every single time. Screw anal people I had one on my team and finally broke him. It’s just like a horse . Then when they realize their comments mean more work for them they will leave less. When you comment on my fucking white space I am going to be an ass back. No one cares and IDEs today will help you navigate code no matter how it’s written. Don’t be a bitch . In all seriousness I feel your pain. They leave or change eventually.
me_again@reddit
Either this is Google or more likely a company who really want to be Google.
MiddleSwitch8@reddit
Your PRs are large enough that there are more than 100 meaningful lines to comment on? I know reviewers being anal is annoying but smaller (and perhaps stacked) PRs can help you help yourself a lot.
uppers36@reddit
My previous job was very similar to this and I’ve had a lot of anxiety over it thinking I’m just a shitty dev. Thank you for making me feel a little better
New_Firefighter1683@reddit (OP)
Heh. I had it pretty good up until now. Mass layoffs at a faang hit me last year and I'm stuck here now
Cokemax1@reddit
It's always shitty manager. or people. don't be fooled by them.
desolstice@reddit
I’ve had a lot of luck dealing with people like this by getting them to write up a standards doc. PR reviews turn into either it is actually wrong/bad vs code style. And any style comments should contain a link to the docs. If they do not exist in the docs then it is personal style that shouldn’t be blocking PRs.
wormhole_bloom@reddit
we use to work like that in the past, until we realized that it was slowing things down
we eventually decided that there are things in a PR that can't block it and it will be up to the author to follow it or not, in accordance to deadlines and etc
it's not ideal to leave things in an unideal way, but it's better than not shipping code and falling behind with our customers
what is the policy behind PR reviews there? is it company policy to follow everything pointed out there?
how is this for other team members? do they also deal with big nonsensical reviews? if yes, do they follow or they ignore these reviews?
New_Firefighter1683@reddit (OP)
No policy. I think the general trend I see is "just do it". I try to fight things that are extremely out of scope and time consuming.
About the same, but since I review theirs, I only point out breaking things or if I see something completely nonsensical or just bad code smell. Otherwise, I'm good with it.
That halves about the PR comments. For big nonsensical reviews, they try to fight it, but usually get overruled by EM (who leaves the most comments). They stay until 9PM to do it.
Middle-Comparison607@reddit
The code is not yours. It's the team code. Be helpful in those discussions, and if possible try to enjoy them. It's not your delivery that's going to get late, it's the team delivery. At the same time "rushing things to meet deadlines" is the number 1 excuse for pushing sloppy code that will leave more things to fix later. Signed: proud an*l engineer. P.s: the sooner your learn not to leave tech debt for tomorrow the better
Cokemax1@reddit
So your manager make comment on your PR. and he is blocking until you make changes?
Ask his permission on extended deadline. you can cc'd PM if that is case. like
"I fully agree with your suggestions and am ready to apply the changes. However, I will need an additional two days and request an extended delivery date. Could you please confirm this? Also, could PM [Name] confirm as well? I will proceed with the changes only after receiving both approvals. Thank you."
armahillo@reddit
you can say “anal” on reddit.
Sonic__@reddit
yes please for the love of god. Can we please stop censoring stupid ass shit. This is reddit. We can swear here. And this isn't even a swear. God I hate this new way everyone wants to speak..... Like I'm gonna be offended to see the word anal, not even used in the context of an ass.
New_Firefighter1683@reddit (OP)
I did in the post ;)
BlackDeath3@reddit
The context just makes it goofy too. It's like censoring "Dick" when referring to a Richard or something.
muuchthrows@reddit
Who is determining it’s a hard deadline? Who is making the decision that X must be delivered to Y customer by tomorrow? Apparently not the same person who sets the engineering standards and culture for the team?
How can you have a hard deadline for something while the rest of the team don’t?
New_Firefighter1683@reddit (OP)
Yeah this about hits the nail on the head. EM sets deadlines on things. Skip has a deadline on another thing. 2nd skip has something else on their mind. PM fights everyone. CTO changes his mind on something and overrides everyone.
I'm trying to make this work
Choperello@reddit
Is it happening just in your PRs or everyone's PRs?
New_Firefighter1683@reddit (OP)
Everyone. People stay until like 9PM to do it. I refuse to
Sonic__@reddit
I guess it probably heavily depends on the company. But when shit got crazy like this and I just straight up refused to work 3 weekends a month to get shit done on time, and also told the rest of the team this was bullshit. I eventually got moved to another team. It ended up being, for me at least, the best thing to happen to my career.
Choperello@reddit
Maybe something to bring up with the team lead then
CoolBeanziesXd@reddit
If I ever see 100 comments on my PR I'm closing it
ryuzaki49@reddit
Easier said than done. And what would that accomplish?
Ok_Cartographer_6086@reddit
100 comments means your PRs are too large. Work in your branch and make small PRs to main with polished code. Keep merging small, non breaking code with tests until your final feature merge is tiny and uses code already merged in.
As a leader leads, if i see a PR with 100 comments and enough code to warrant it - i'll block the PR outright and tell the dev to break it down.
New_Firefighter1683@reddit (OP)
Would love having smaller PRs, but you know what happens when you have 4 PRs?
It takes forever to hunt people down to review it. 1-2 hours easily. 3-4 PRs? that's 2 days of hunting people down to sit down to review it.
HenryJonesJunior@reddit
What does "hunting down people to review" mean? Does your team not have a code review alias/queue to assign things to by default?
Waiting for code review shouldn't block you. Work on a chain of CLs. You can even send them out for review in parallel.
revrenlove@reddit
Arguably, any pr with a hundred comments is a pr that is too big and should be split up into smaller stories
sickcodebruh420@reddit
There are a few separate issues here: code quality requests, scope creep, architectural changes after there’s already a plan set, documentation demands.
Code quality: Legit or not, I usually suggest scheduling a pairing review session with someone when I or they call out a lot of these. It’s usually faster, feels less aggressive than a wall of comments, yields to better results with less back and forth, and forces someone to contribute to the solution or STFU.
Scope creep: you can push back whenever that happens, make a new ticket, bring it up in next sprint planning.
Architectural changes after there’s a plan: sometimes things just don’t work the way they’re sketched out. If it’s happening a lot, schedule a call to review, involve manager, understand if the demand is valid or what went wrong in planning, and update expectations accordingly.
Documentation demands: this should be documented as acceptance criteria on the tickets and accounted for in sizing. It shouldn’t be a surprise. Bring it up about in sprint planning.
This would all drive me insane. Good luck. Hope you’ll share updates. 🫡
New_Firefighter1683@reddit (OP)
I've been doing this as much as I can. Problem is... nobody ever has time until 8PM. I'm not staying til 8PM.
When I do actually get the sessions, it goes well. And typically... they change their minds and dismiss their comments, so I try to do it as much as possible.
pragmatica@reddit
Do they do it to each other also?
This might be some kind of hazing.
If this doesn't happen to anyone else's PR you're looking at harassment/bullying.
Then it's time to get HR/management involved.
I have a funny story to tell about a CTO who thought this was ok and is now spending more time with family in his wife's native country while he figures out what's next.
Homie don't play.
New_Firefighter1683@reddit (OP)
Yep, another person's PR has like 50+ PR comments right now. But only because I'm not going crazy with it because it looks fine...
kolima_@reddit
Introduce kick off meetings you do a design session ahead of time with the devs about how to gonna implement the feature, document this and this is what they get. If they want more after thoughts about it gonna be tech/refactor tickets
New_Firefighter1683@reddit (OP)
I mentioned in my post I already did this. We were all aligned.
People suddenly started saying the opposite. We had notes from the meeting. Pulled it up and suddenly everyone literally said they changed their minds.
edgmnt_net@reddit
I can't tell based on the story alone, this could be either good or bad depending on specifics although the figures you provided do seem rather worrying. I'll argue a bit for the good side here since it might not be obvious...
Are your deadlines really that strict? What are the expectations? Many jobs I've been in rarely upheld them strictly as long as there were real issues getting solved and not just laziness or stupidity at play. Or at least someone stepped in and said "we really need this, so we can ignore those concerns for now". In any case, unless you ignored some best practices, guidelines or the style in surrounding code, they really cannot be that strict because you don't have much control over it. Not so soon, anyway, because given some time you might find out there's some consistency to their preferences.
I've seen both strict and loose reviewing policies. Open source can be very strict and the average project in a feature factory can be very loose. I also generally welcome people pitching in to review stuff because most tend to stay away from things they're not directly involved into.
You may have secured agreement but issues were yet to be discovered with that approach. This could be a miscommunication issue (not necessarily on your part, perhaps it's the team barriers) if not enough eyes were involved in the first place or something more benign. Nevertheless it might be nice to actually iron out the important bits and maybe rework the entire thing if it's too deficient. Initial agreement isn't everything, although it might take you off the hook. Again, in open source I've seen project leaders saying "this isn't getting in until we find a better way to do X for everyone involved because it's a serious pain point". Honestly I'd rather have that than the more usual situation where everybody does the bare minimum to avoid missing deadlines and tech debt just piles up.
Context switching to different tasks may be expected, assuming progress is being made and you're not just collecting lots of unfinished tasks that never get finished due to the other points. I personally enjoy some variety and it's how one gets to know a lot of other stuff (which facilitates further involvement).
As long as there's no serious technical conflict on how you see things, some reasonable alignment may be possible. How do the others see the process? Do others get to a point where these things go reasonably smoothly?
arlitsa@reddit
https://www.netlify.com/blog/2020/03/05/feedback-ladders-how-we-encode-code-reviews-at-netlify/
That combined with “that’s a great idea, I’ll log a ticket to address in the future”
I lean more on the latter. We try to do the feedback ladder but people don’t always adhere to the practice.
FrequentReporter9700@reddit
Op don’t be pleaser you can’t make them all happy and in the end you’ll be the asshole. If you think their comments are not relevant or outside of the scope just tell them and backlog their wish and move on. Not all the time but when you are fully confident with your code stand your ground and refuse to do with their way by justifying your way.
Aggressive_Ad_5454@reddit
Suggestion: Say to your manager.
No-Economics-8239@reddit
I mean... do they have documented style guides and naming requirements? Learning that kind of thing from the review of a PR makes no sense to me. Why would they unleash you on their code base without any training or on boarding first?
It's not completely unusual for tribal knowledge to accrue and become part of the team culture. But it seems completely feral behavior to distill this information on PR comments.
I would want an in-depth explanation as to where these requirements are coming from and why they are so important. And if I don't get a comprehensible explanation and agreement from team leaders, then I'm going to denounce it as nonsense or heresy.
Aggressive_Ad_5454@reddit
Suggestion: Say to your manager.
pukatm@reddit
Distinguish between what can happen now and what can happen later. Tell the reviewers that you created a ticket to address the suggestion which can happen later and link to the ticket. If it is a 5 minute change or if it will save you a lot of work down the line try hard to get it done now, otherwise you will just develop a reputation of being difficult to work with or producing bad quality work. Even worse it might make the reviewer stop caring.
Main-Eagle-26@reddit
Sounds like some engineers who are way too navel gaze-y and bikeshed everything.
When this sort of thing becomes unreasonably pedantic, it often correlates to a very very career-limited engineer. I never see these inflexible types ever move beyond senior if they even get that far.
Fabulous-Arrival-834@reddit
Only one solution - Call this sh*t out in emails and team meetings. Data is your friend. Show how much time is being wasted on blocked PRs and show them the data on "constant refactoring resulting in diminishing returns".
Create a coding standard for the team and make sure they align on that. If they say in the future that you shouldn't code like this - point them to that coding standard and put the onus ON THEM to make changes to the coding standard first and get it approved before you can incorporate their new coding style.
Once you start putting action items on THEM, they will realize that they are the ones who have are blocking the PR. When they start getting held accountable, they will stop putting these roadblocks in your way.
The only way out of this is to play their game and beat them in it. Make sure they can't just put in comments and go about doing their work while you are blocked.
KaleidoscopeSenior34@reddit
Some orgs are true engineering orgs others are true product orgs. Looks like they value correctness over delivery speed. This is the culture you've moved into moving teams.
Careful_Ad_9077@reddit
Who is the boss?
Eitehr they are gangin up on you and you need out, or your code style is not aligned wiht the team's and that is something that needs to get fixed.
besseddrest@reddit
find an eng on the team that you generally work well with and ask them to show you how they navigate their daily tasks
you might find that you need to make some adjustments in areas that you didn't think were problems
you might have some hard stances on the way they approach things but the overall goal is you want to understand how another engineer on the team is able to take their tasks from A to B with the least amount of friction, and you want to try to mimic that
AromaticStrike9@reddit
Lots of lube and go slow.
ZealousidealReach337@reddit
Inch at a time
karambituta@reddit
Estimate ticket taking all of this into account. Eot