Senior Developer is very protective of the codebase and very picky with code reviews. How common is it?
Posted by Relevant_Mortgage349@reddit | ExperiencedDevs | View on Reddit | 134 comments
I am also a senior who just started. One of the senior devs I am working with has very high standard for a code.
Its very small startup, where he was one a developer #2 and developed most of it. Although his code is far from perfect, I am not skeptical about it, just dealing with a situation and keeping the same style.
Looks like he wants to control what goes in very religiously. How would you deal with it?
snuggly_beowulf@reddit
Give an example of "picky".
Relevant_Mortgage349@reddit (OP)
For example, last week I got rid of iframe and built a custom solution. But he turned it down by saying that most frontend stuff was written using iframe so it’s better not to touch it.
Iframe is not necessary a bad solution, but this specific case could be implemented without it. It wasn’t by intention to change it, but it was a quick fix that didn’t take much of my time and didn’t affect anything: all tests are passed with even better performance.
NastroAzzurro@reddit
Do you guys communicate as a team on what’s important and what’s on your roadmap or do you just submit PRs with fixes because it’s a “quick fix”. The way you are describing how you submitted your work is a red flag to me and there may be merit to the senior’s decision.
Relevant_Mortgage349@reddit (OP)
I am fairly new, just down the best I could to deliver things faster and better.
NastroAzzurro@reddit
Especially if you’re new that’s a fantastic strategy to build resentment from your peers.
jlstef@reddit
I'm so sick of this toxic mentality. It's been in the industry at least for the last 10 years.
Why can't we let people excel?
Don't really have confidence this person understands all the implications of the iframe.
But who cares if one colleague is fast?
Echleon@reddit
People can excel. Going around team processes is not excelling.
jlstef@reddit
Can they tho? Some people aren't going at breakneck speed to show people up, they are just fast in general. Not referring to process here.
Echleon@reddit
No one is complaining about people who just do their work quickly. They’re complaining about people who think they know better than everyone because they’re strong technically, despite just starting out at the company and therefore, missing a lot of context.
TokenGrowNutes@reddit
No point in engaging the toxic wannabe senior.
jlstef@reddit
People at companies can be wrong. Why is everyone so damn insecure.
It would have been easy to clearly communicate to this junior person and say hey, we do this because xyz. It's called mentorship.
And perhaps that's what the other person meant. But they specifically responded to the comment about trying to have high productivity at a job. And I've seen high productivity punished so many times.
Porkenstein@reddit
I have seen many high productivity new people believing they know better than incumbent staff get in conflicts. The ones who can back up their assertions that changing things for the better is worth the time effort and risk are respected and given the opportunity to improve things. But the vast majority of them are just overly enthusiastic and putting effort in the wrong direction.
Also it doesn't matter if you're right, if your team isn't on board you can't do things unilaterally. it's more important to have a functioning team than it is to have slick code.
jlstef@reddit
That's great if people actually stay open-minded. Unfortunately, too often, they don't.
And what is over-enthusiasm? Been hearing this for a decade. If people are motivated to grow and learn, why is that seen as bad?
Porkenstein@reddit
Over enthusiasm is "your code is bad, I'm going to rewrite all of it!" or "this company does X terribly, it should be doing it like my previous company!" Fresh ideas and perspectives are great but when people who understand why those things are the way they are temper hopes and expectations, that new person can either look into it and plan or pitch the changes carefully (rare but extremely valuable), drop the ideas altogether, ignore warnings and try to make changes unilaterally (bad), or constantly complain without trying to understand why things are the way they are.
jlstef@reddit
That's not the same as over-enthusiasm. That's a lack of realization about the implications of context.
Which is, again, an education opportunity. Really simple to fix. Just have a conversation about the complexities of the codebase and ramp the person up. If you don't have time for that, have a conversation about the meta. But don't label it as being over-eager. That's an innacurate label and creates bad communication.
Echleon@reddit
No one said that people at companies can’t be wrong. The issue is coming in and changing something that didn’t need to be changed without talking to the other senior dev.
jlstef@reddit
Eh, I'd be glad if a junior dev took initiative to clean things up.
Defensive, kiss the ring vs mentorship and clarity.
If there are reasons why it shouldn't be changed, or why resources can't be wasted like this, it's a great time to talk about the meta of tech. But people never do that, they just get pissy. It's pathetic, actually.
Echleon@reddit
The time to do that is before just changing the code though. Imagine OP realizes the other dev was right and so he switches it back. He wasted that time, just to end up where he started, instead of discussing with the other dev the pros and cons ahead of time.
jlstef@reddit
But... That's a teachable moment, no?
Why get mad because they didn't know before they were taught?
Echleon@reddit
The anger isn’t at them not knowing, it’s at them not asking first. OP could be 100% right about his implementation, but he’s in the wrong for not discussing with the other dev first.
jlstef@reddit
That is what OP didn't know-- to ask. The meta. So what if they are wrong? Hence a teachable moment.
Software isn't some military hierarchy of follow orders and if you step out of line you get demerited. Maybe they teach that in the military, but in the civilian world, people are trained from before they enter the industry that initiative is a value. If you want command and control as a norm in your workplace, then that should be made apparent during interviews and cultural training during onboarding.
Echleon@reddit
They’re a senior dev and should know better. No one is talking about some militarized like hierarchy. It’s just common courtesy to discuss stuff with someone who’s been around longer before just changing stuff.
jlstef@reddit
Depends on the culture. Not every workplace is like this. Sometimes people just have an open and pragmatic mindset of clean as you go and people will submit PRs out of nowhere. Some teams take the tact that it's good learning. Again, comes down to norms. Used to have a great scrum master who did team formation and would elevate and call out norms on purpose.
Ciff_@reddit
It is up to OP to listen and ask questions to learn how the team and workplace works. And if he takes initiative and prefferes to do so without asking, that's not the end of the world, but expect feedback such as that it was the wrong thing to do.
There are many reasons this particular scenario may be wrong. There may already have been a discussion on how to proceed with the FE architecture and when. It may not be an big enough improvement to introduce a diverging pattern. And so on. It is up to the senior to explain, but also up to OP to ask.
jlstef@reddit
That's one way of framing it.
Or, how about they are all on the same team together and everyone gets a voice in terms of how they collaborate?
Power kabals and following existing norms have their place. But so does open collaboration. Becoming a dying mentality.
Everyone in tech now is so hierarchical and cagey. We used to be collaborative and pragmatic. Not anymore. People clutching their coins.
Ciff_@reddit
Yes. I don't enjoy the strawman.
You are seemingly missing that there likely already exists agreements and wow in place, just like there exists code standards and architecture in place, and they do so based on precious collaboration and discussions. That does not mean theese cannot be changed or discussed. Noone is saying that. It however does mean that they are followed untill you together as a team agree about something else.
Just because a new member enters the team does not mean whatever exists are thrown in the air. That would be an issue for collaboration, democratisation etc. Noone is saying OP does not have a say.
jlstef@reddit
New team members aren't mind-readers. These existing norms are very often not communicated clearly.
And even if they are, this defensive and entrenched mentality maybe should be shaken up by someone new.
This is just basic human dynamics. You can try to suppress the fact that groups change every time you add or remove people by just insisting on a culture, but that doesn't really work in reality. This is why there's so much high turnover. Joining a team is like getting past the honeymoon phase of online dating to find you both had very different pictures in mind.
Ciff_@reddit
I have already said it is a shared responsibility. From the existing team to communicate expectations, and front he new member to ask. There is no way to communicate ever detail you have to do it as you go.
High gears for getting feedback on a PR. Ask why instead. It is, as I said, a shared responsibility.
Group forming dynamics does not mean you throw code standards, architecture, wow agreements etc out the window.
jlstef@reddit
Sure, all that sounds reasonable.
If the person makes a big PR and that isn't ok with the team, then talk it out and let the new person have a say and actually listen to their POV/philosophy. Wrangle honestly with their POV and continue to communicate.
The end. Lol
Ciff_@reddit
I men it is basicly my interpretation what happened. He got feedback on his PR but expected standing ovations. If you don't agree with or understand the feedback, ask questions and discuss!
datacloudthings@reddit
fast ain't everything
itb206@reddit
The point was as a new guy you need to build credibility before running train on a codebase.
jlstef@reddit
"running a train" right. Ok. More kiss-the-ring. More dont-trust-outsiders.
This is defensiveness and fear of losing jobs by way of obstructing progress.
itb206@reddit
If you're new to a job you spend a few months showing to people you are actually as good as you think you are. They don't know you or trust you and why should they? It's not about obstructing progress it's making sure you're not a moron who will have large negative impacts.
jlstef@reddit
The proof is in the actual work. And -- proposals are part of that work.
What is this kiss the ring nonsense?
itb206@reddit
No offense but I've always been a super high performer, quick work, rise up to lead architecture and large projects etc. You do you, but you sound painfully bad.
jlstef@reddit
Apparently I share your bio but not your toxic mentality.
tristanhall@reddit
I’ve observed this several times with new engineers. Rather than shut them down bluntly or reject it without explanation, I think the best approach is to recognize the value and/or intention of their change, and then direct them to campaign for the change and get buy in from the rest of the team (potentially managers/leadership if it’s significant enough), and offer to help them tune & sell their solution.
New/junior engineers can still have plenty of great ideas that challenge the status quo in great ways, but I think the hardest work (and usually most rewarding) is evangelizing an idea and getting a team/department/company on board with your solution. Having a million good ideas aren’t going to be much good in a team environment if nobody agrees with you.
Side note: there are limits too. This isn’t to say all changes are innovative or good. Some PRs are just bad and shouldn’t be merged for the sake of the system and/or the company. And, if I’m having to direct an engineer more than 2-3 times to go evangelize/get buy-in for their unplanned change that isn’t on our roadmap, then it’s going to be hard for me to consider future requests because they clearly aren’t learning.
jlstef@reddit
Yep. And if you want to have an ask-first culture, set expectations. Don't punish people for not playing by invisible rules.
DarkGeomancer@reddit
If you're new just follow the flow, try to be proactive but don't create tasks for yourself and just try to help the team as hest as you can. And you do that by not rocking the boat while you're still integrating.
enHello@reddit
Everyone here defending the convention of using an iframe because it was part of the codebase. But, I’m curious, what does the iframe do in this example? Because imo the only time to use an iframe is when you need to embed content from another site onto the page. The are some edge use cases otherwise but they’re edge (e.g. I once worked on an iframe authentication app that both mobile apps and desktop app used for authentication, but that was mostly because the dev spearheading the work knew web more than swift and Android.). Otherwise if you can’t avoid using an iframe, do it.
fdeslandes@reddit
It can be the right solution when the (heavier) alternative is using micro-frontends. As an example, we have a custom platform that we use where I work, but it is driven by clowns, full of bugs, not properly updated and without dependency management. Basically, it's a clusterfuck.
All teams across multiple unit that have to integrate modules in this application are better off adding an iFrame in the original "portal" and developping their app within it, separated from the clusterfuck as much as possible. Every important/sizeable project within it ended up in an iFrame because it's better than the alternative (not being isolated from the clusterfuck).
dezsiszabi@reddit
Some web based rich text editors also use an iframe to avoid style/script pollution from the surrounding app. For example TinyMCE (though it can be configured to not use an iframe as well).
JobSightDev@reddit
Using an iframe is certainly a head scratcher, and it's probably a good fight to fight... Someday. Not on your first week.
AaronOpfer@reddit
So you changed something that you didn't strictly need to change while working on something else, in the same PR? (The rest of my post is assuming that answer is yes. If no, disregard)
Having been in these shoes before, it's possible that the senior has a lot on their plate, such as their individual project work that they need to complete despite the fact they're blocking their teammates by having not finished their code reviews. Seniors may simply just have limited ability to be receptive to something unexpected showing up in the review queue when they're trying to unblock people so shit gets done.
To respect everyone's time and mental energy, you should try to keep PRs focused on exactly the stated requirements, and try to put "serendipitous" improvements in separate PRs and mark them clearly as nice-to-have, non-blocking PRs. When the senior is ready to be receptive, they can come read your PR and, if your change is rational and clearly explained, will likely accept it. If PRs are short and focused, the senior will be better able to prioritize unblocking the most business-critical needs in priority order without having to revisit PRs by having discussions about irrelevancies.
There's other issues with having extra stuff in PRs since reverting your changes in the chance of a break means reverting your "extra" code too, which can be a big problem if someone else hacks on your changes.
jmking@reddit
This is it exactly.
Also I would advise OP to not make assumptions based on the idea that the original authors were just incompetent. There are good reasons to iFrame things depending on the context (security is a huge one if there's mixed 1st and 3rd party code on the host page for example).
Another thing to avoid is partial refactors. There's nothing worse than a codebase that has 10 different partial refactors because someone thought something was "faster and better" but didn't follow through on completing the refactor across the rest of the codebase.
Even if it were objectively better, I wouldn't accept a PR that makes an architectural change like that with no prior discussion and no plan to see it through fully.
datacloudthings@reddit
Gosh I wish I could update this more.
olssoneerz@reddit
So did you just cowboy your own solution? You could’ve coded up the best piece of code in the world but it would still be frustrating cause it sounds like: - You weren’t tasked to do that. - You didn’t communicate any intent to do that before doing that.
nickhow83@reddit
Quick to fix, maybe. Makes things faster for you, maybe. Means you get something done quickly and can show progress, maybe.
But now you’ve made the architecture of the app inconsistent which means it slows other people down. It also introduces a need to regression test whatever you’ve done in a lot more detail than the scope of the ticket you’ve done.
Then there will need to be a discussion about how to move forwards. You’ve come in with a new solution, that is on the surface faster, but now do you want to change the rest of the iframes in the app? It’s a common theme for newcomers to want to rewrite things, because it’s not what they’re used to and there’s “a better way to do this”. I used to be as guilty of this as anyone.
However…
Inconsistency and going fast can often lead to bugs more than the existing solutions put in place.
I see you say that all tests pass, but what if you’ve now introduced an edge case that wasn’t previously considered?
It’s really better to discuss these things to understand the why, rather than just doing them.
It upsets me how us developers feel the need to deliver, deliver, deliver, faster and faster. Slow down, take a step back and ask is this the right thing. Talk to the team before making what is effectively a fundamental change.
nutrecht@reddit
These kinds of changes you discuss up front. You really need to communicate better. Our job is all about communication, and bad communicators make bad software engineers.
evanthx@reddit
If everything is in Iframe everyone can support it. You go do some custom stuff for that one thing, and everyone has a maintenance headache until someone finally goes and fixes it to be Iframe like everything else.
“It worked slightly better” isn’t going to make the guy happy who’s stuck trying to figure out what weirdness you did, you know? Even if you did a GREAT thing - don’t forget someone is going to get stuck trying to maintain it.
Worst case is you go do custom stuff in a lot of places. What’s lets is an unmaintainable MESS. Even you won’t be able to maintain it, once everything is different and there is no standardization you can’t remember what you did where, and it just gets UGLY.
JobSightDev@reddit
So basically "either way is fine, but I'm going to change it just because and he didn't like that"?
Relevant_Mortgage349@reddit (OP)
It’s not “just because”. I was genuinely caring.
JobSightDev@reddit
Right, I get that.
But i think you are downplaying the importance of conventions.
There are a LOT of sections of my code that probably aren't the most performant, but we follow a convention so that it's super easy to get in and see what's going on.
It sounds like they've adopted a convention to use iframes for specific parts of the code. Removing this convention is just going to cause confusion for the next dev that expects to find an iframe.
Relevant_Mortgage349@reddit (OP)
Clarification: the code has both iframe and custom code. Iframe is mostly faster to implement, but in my opinion it’s not optimal solution.
In general iframes are considered a bad practice.
Agifem@reddit
A code is written once and read ten times. I'll take readable and less-than-ideal over best and complex, any day of the week. Maintainability of a code is a real problem, and conventions is a real solution to that problem.
JobSightDev@reddit
Also, you don't deserve the number of downvotrs you're getting here.
I feel like I'm on stack overflow or something!!
fasync@reddit
He gets the downvotes because in the meantime numerous explanations have been given for the senior's behaviour and his mistake has been pointed out to him, but there is zero insight. Tech can be the happiest place to be - but not the "I do what I want because I know everything better" place.
Relevant_Mortgage349@reddit (OP)
And they say tech is the happiest place to be lol
neuralSalmonNet@reddit
my suggestion would be to keep a personal WTF list - things that don't make sense, things would be changed, improved etc. Fresh eyes are great at noticing bad things that others have gotten use to.
in 3 -6 months revisit this list, bring it up with people you learned in that time that would care of changing it and then have the conversation.
pauseless@reddit
I mean this with the best of intentions: you sound stuck in your opinions. Flip the positions:
You have built a product, it’s not necessarily perfect, but time and effort has been put in to it. A new dev arrives and inmediately wants to use an iframe for a solution. You know iframes = bad, but you can’t really explain why, and the new dev is right that it’d speed up implementation of that feature.
Best you’ve got is “that’s not really how we want to do things going forward”. The new dev isn’t satisfied with that, because they can point to examples where the pattern hasn’t been followed before.
How would you feel? Would you be frustrated that the new guy is trying to reengineer things from the start? I would be.
I can give you a story from my own work: I wrote my own SQL migration library, it was designed to address precisely the issues we had faced using generic tools within the context of our deployment setup. I spent half a day on it (no joke - extremely specialised tools can be quicker to write than learning a generic tool) and it never failed us once.
A new, very smart, dev came in and immediately took me to task about it. Honestly, I just had to say that previous projects had tried other migration libraries and they all resulted in more pain and specifically made it far too easy to break prod. My design basically guaranteed git conflicts when multiple branches were all adding changes at the same time, forcing a human to find a sensible ordering.
I always tell my employers that the first three months I won’t try to change anything. Even if I’m being hired as the very most senior dev, expected to bring my experience.
Coming in guns blazing is just an awful approach. You may see it as being keen and wanting the best, but imagine it from the other side. It’s a lot to handle a new person just going off and doing whatever they think is best, with no context.
My experience is that many of the bad practices you immediately notice in the code, aren’t actually that bad once you’ve spent some time in it.
PhoenixWright-AA@reddit
It’s fine to believe this but your mistake is not reviewing the choice and getting alignment before coding.
Thefolsom@reddit
I'd suggest taking time to understand why the team/organization sticks to certain conventions before rocking the boat. There's always some new "better" way to do things, but unless you have a solid plan for refactoring the existing usage (which you simply aren't going to know 1 week on) then all you're doing is adding complexity.
JobSightDev@reddit
I don't disagree with you, but your first week is not the time to fight this battle. And especially without talking to anyone.
You simply don't know enough about the history of the product and the history of the team to make those kinds of decisions in your first week.
Get a couple of months minimum under your belt first, then you can discuss changes like this.
Yodiddlyyo@reddit
I hate to sound negative, but "Genuinely caring" is generally not a good reason to change an implementation.
There are very specific company/team structures where it is ok. If it's a startup and you've been told "all improvements are welcome, just fix stuff when you see it" , or a project that is "get a demo out as fast as possible".
Otherwise, just changing an implementation for no reason is considered bad practice.
And I say no reason because the only reasons are
That's it. Refactoring things makes it harder for other devs because then they need to learn the implementation. Or maybe something exists a certain way for a reason that you can't see due to your newness. Or maybe it's part of a future change that's going to be reworked, and you just wasted your time.
Changing a few lines here or there, no problem. Rewriting an implementation, not great.
So, not saying what you did is horrible. Unfortunately, you're in a position that doesn't appreciate "just go fix it" and that's something you will need to be ok with.
Soccham@reddit
But unless you’re doing it everywhere, it’s now another pattern and additional complexity being added to the code
Relevant_Mortgage349@reddit (OP)
Complexity is not an issue here. Code has both iframe and custom solutions. So it’s not standard across all codebase, per se.
NiteShdw@reddit
As a new person, you don't know the history of why things are done the way the way they are.
Please don't make refactor without understanding the problem and running your idea past existing developers.
It may be tech debt but tech debt has reasons. You don't know what you don't know.
I have 20 YOE and I don't even suggest any reflectors for the first 3 months and after that I start with small, low risk refactors before finally moving to bigger ones.
Younger me was just like you. I learned.
software-erosion@reddit
I agree, this is an important mindset to always have. I do my best to not judge any code I see at first glance because of this.
It's hard to resist when you're digging through some messy legacy code that seems all wrong, but the reality is that you just don't know the "whys" of everything. The code has history and it is built upon years and years of decisions that were directly influenced by requirements, constraints and other limitations.
If you don't know the history of the "whys" then you should be asking questions first, and refactoring later.
jxj@reddit
i don't love iframes but i'd prefer consistency. wait more than a week before trying to introduce fundamental changes to established patterns in a codebase. and even then, don't just do it in a pr. bring it up in a conversation first, get buy in, then make the change. if you need to show code, it's fine to make a branch to show how your solution compares to the existing pattern
cougaranddark@reddit
So, now you have something that is not an iframe where everyone expects an iframe to be. QA has no idea to test it because it's not an expected change in the ticket. Eventually some code designed to affect all areas where an iframe is won't apply to this one area. Other devs coming in expecting the same convention now stumble upon something different that uses conventions that were never agreed upon. Soemthing could break in some user's browser that you don't know about one week in. You went beyond the scope of the ticket. That was a very junior move. If you want to have impact, learn how to build consensus.
gfivksiausuwjtjtnv@reddit
As a tech lead IMO killing the iframe is the right decision at face value.
But - if you’re making decisions at face value you might be missing some nuance that a dev who maybe spent a while working on the business and codebase
Did you ask why the iframe wqs used ?
Do you know the full details of why it was built like that - did you actually ask the other devs in slack?
Just spend 2 minutes flicking someone a question if you think of an extra fix that hasn’t been scoped out, if you’re unfamiliar with the ins and outs of it. Just communicate. This goes 100x if you’re new to the team.
jay_boi123@reddit
Not entirely well versed in front end stuff myself, but if you believe strongly in your solution I think it’s better to have some sort of a knowledge share on best practices for your domain.
During this knowledge share your team can formalize coding standards, guidance on framework usage, design patterns, and other important considerations.
Then when you establish your argument there in a document you can add it to the readme of your team’s repo. So the next time you feel like the senior dev is being nit picky you can point to the doc.
adh1003@reddit
TBH that sounds like you ignored all idioms in the code base, inadvertently violated "principle of least surprise" and did your own thing regardless.
You claim it's better, and it might be, but that wasn't the place for it. You introduced tech debt by doing things in a different way from everywhere else.
If you want to refactor your code base to remove whatever the hell
iframeis doing in there regularly in the first place (the mind boggles!) then that's a discussion with the dev team, the person earmarked most senior to make the call on yes or no, then estimate the implementation time to do these changes across the whole code base, get it in the prioritised backlog and go for it once it's considered the right time.If it really is simpler and faster then I'm sure, if time and budget permits such a sweep, that people would go for it. But piecemeal doing different for sake of different is just really annoying.
I have a feeling I could exactly be that dev you describe and I think I lean towards sympathising more with them than you at this point, BUT that's only on the basis of the (understandably, assuming closed-source and company confidentiality) vague description in this example.
TokenGrowNutes@reddit
Your first week and already complaining? It’s good to have standards.
Based on other comments like rewriting that iframe: It seems like you probably aren’t communicating any important decisions. First week. Chill out and only change what’s being asked for.
Im2bored17@reddit
Or at least ask about changing it before you go changing it.
Suggesting changes to big things is not necessarily bad, sometimes it brings new ideas / approaches. But as the new guy, you don't know all the context / history that may make your idea really stupid.
TokenGrowNutes@reddit
Oh, for sure, always push for improvements. If anything, just asking why all the iframes would have been the fairest way to start.
But don’t hero on day zero.
I don’t think this is gonna last, though. Already complaining, ffs.
Haunting_Welder@reddit
If he developed most of it, he should have most of the control
Existing_Station9336@reddit
If I was new I'd do my damn best to write code that looks exactly as if one of the existing developers wrote it. My commits should be indistinguishable from commits of the existinging developers. By doing that I am sending a clear message "I am one of you". Only AFTER I establish myself as a developer on an equal level I'd start carefully (see others comments on how) bringing in improvements.
RegularLeg7020@reddit
4 ,6 r tdr,e6y57yre Hi Team, I ha😃ve some urgent personal work today from 2 pm to 5 pm. Please expect delay in26s response.https://maps.app.goo.gl/Hx6edQschzQwPBZN8Hi Team, I have some urgent personal 2wo5wq rk today from 2 pm to 5 pm. Please expect delay in res51ponse.Hi Team, I have some urgent 3personal work today from 2 pm to 5 pm. Please expect delay in e.
wrex1816@reddit
The fact this is a startup and he was #2 says it all. Have been there with these guys. They end up founding startups because they can't work with people at a normal company, make terrible coding and architecture decisions at the startup, then act like a dickwad to everyone who joins after them acting like you're too stupid to realize the brilliance of everything they have done (which is infact, a giant mess). Everything you do is nitpicked, while the system he built is held together with strong and bubblegum, but don't dare point that out or he'll cry to #1.
He's protecting his turf because I'm certain he tells everyone he's "#2" and needs to feel big and important. You won't change this guy, If run far away.
datacloudthings@reddit
Consistency in a code base is a good thing, and having a "house style" is a good thing.
Getting to that can be politically challenging. Maybe the best way is to have someone be a benevolent dictator.
RegularLeg7020@reddit
The problem is finding the benovelent dictator... Most of them dictators are like Persphone and Merovigan
datacloudthings@reddit
power corrupts, etc.
wknight8111@reddit
Following the Principle of Least Surprise I find it's usually much better for a codebase to be consistent with it's code formatting rules, patterns and organizational structures even if those things aren't your preferred way of doing things, or don't follow "best practice".
I've worked with a lot of devs over the years, professionally and in several years of open-source software I've seen a lot of developers get very protective of a codebase. There are many possible reasons for this, and you can't diagnose or address the issue without getting to know your coworker better. Some of the possible reasons that I've personally witnessed are:
There may be others as well. Speak to your coworker about it and see what's up. If you have suggestions for improvements do present them but be prepared to justify yourself and even show some evidence to back your claims. Also, if you want to change something be prepared to do it completely. A lava flow anti-pattern will turn an imperfect-but-consistent codebase into an absolute mess of half-baked ideas and surprises everywhere.
Impossible_Cup_7358@reddit
I’d look elsewhere, not worth it
react-rofl@reddit
Ego is the enemy
SomeAnonElsewhere@reddit
If you don't disagree with most of their points, just code to that style. If you disagree on something, then bring it up. Working on a team requires communication, and you're literally getting paid to work through every little issue with them. I think it's cool that they care. Many don't and will rubber stamp anything.
DualActiveBridgeLLC@reddit
Yup. I had a guy like this and it forced me to really decide on what I cared about. It worked great, codebase was clean and he knew that when I raised an issue I really believed it was a problem. Obviously this won't work if if they never compromise or think they can never be wrong, but I haven't seen that.
Relevant_Mortgage349@reddit (OP)
That’s what I am doing, just going with a flow. But adding small improvements, and obviously I communicate. I brought up my arguments, but it wasn’t enough. I am not sad about it, just want to do better for the team.
iamyourtypicalguy@reddit
My advice OP is to file a separate ticket in the backlog for improvements and have it as a keypoint. That way the team can track it separately and tests can be isolated to that change only. You might not know yet which features are affected by it. Then on your sprint refinement, you can discuss it with the team regarding your findings and have a link to the ticket with documentation. Other devs can also voice out their opinion this way.
RoughCap7233@reddit
The concern could be that any changes made over and above the expected could lead to technical debt (code not following established patterns) or would create additional effort (testing and validation, production issues due to unforeseen consequences etc).
flaming_goldfish@reddit
It sounds like your arguments don't align with the engineering dept's priorities or the company's priorities. The best way to get your persuade your seniors/managers is not by making a purely technical case, but by making both a technical and a business case for the improvement. If the code change doesn't satisfy current business priorities it's not going to make the cut.
SomeAnonElsewhere@reddit
You said it was a small startup. Is it just you and the other guy? What do the other devs think if there are any?
Relevant_Mortgage349@reddit (OP)
We have around 5 devs, but only 2 of us working on frontend right now. I am a full stack, if it helps
OverEggplant3405@reddit
It has taken a while, but I now appreciate people like this. I would say it's not common enough, actually. Plenty of senior devs are asleep at the wheel or too overwhelmed with other priorities to review code. Those who have the time sometimes are conflict averse and avoid bringing up concerns for fear that it will be received poorly.
The fact that he is willing to bring up small issues is a good sign that he will be willing to communicate when there is a problem, which is much better than working with someone who doesn't handle problems directly.
How to deal with it: first of all, I would thank him for taking the time to review my code. Keeping communication open is critical. If I didn't agree with something, I would ask him to help me understand where his opinion came from regarding the change.
If it is simply stylistic change that I don't like, I would most likely do it anyway. Consistency is more important than having the best style. If his change creates problems, I would ask him how he would handle those problems in the code if they were to come up.
Since it's a startup, it's possible that he is used to things breaking constantly and other devs making a mess in the code. That would drive his anxiety around accepting code changes, which would lead him to be overbearing in code reviews. This is a subtle hint that there is a need for more automated testing and quality control.
If it's causing problems, I might ask to meet with him for 10 minutes to get his opinion on how code should be structured. That will help you run into fewer issues in the code reviews, show him that you care about code quality, build trust, and open up communication, all of which are great goals in the first few months of your new job.
AndyWatt83@reddit
I love working with fussy bastards - makes my code / coding so much better!
dacydergoth@reddit
Code reviews and PRs are separate things. PRs are a communication mechanism and you might want to adopt a "Shall Approve" philosophy for PR. Use linters, code analysis, pre-commit and tests as your quality gates. Only block a PR for a critical issue
Then do code reviews as a peer thing with mutual respect as a separate activity
Echleon@reddit
This is silly. If something in the code doesn’t following standards for one reason or another, it should be addressed then and there. There’s a high chance that follow up code review doesn’t happen, and then the shit code makes its way to prod.
dacydergoth@reddit
Ask yourself... What matters more, shit code in production making $$$$ or code sitting on a dev branch because someone nitpicked it?
CheraDukatZakalwe@reddit
I mean if you're going down that route then why was OP doing a solo run, wasting company resources on something nobody asked for and didn't expect?
Code-Katana@reddit
There’s definitely a bell curve of ROI between nit-pick-blocking and rubber-stamping, but I’d still err on the side of caution first.
Far too often the “just get it out there” mentality costs way more $ after fixing the n-th consecutive production bug compared to just delaying a week to “get it closer to right” before initial release.
dacydergoth@reddit
If that week is the difference between your startup succeeding of failing then it becomes critical.
Devs often forget that we're not here to make good code. We're here to make $$$$ for the company. Every line of code we write is a liability, it has to be tested, documented, maintained, etc. The reason why the company tolerates that is because that code makes them $$$$
If you code is sitting in a code review it isn't making $$$$.
This is why we have techniques like A/B testing, canary, feature flags and can revert changes quickly.
Push the damn code. If it breaks, roll it back and do better. Use the A/B testing. Use the canaries.
But code which is not in production isn't earning $$$ and without that you don't have a product, you have a vanity project
Code-Katana@reddit
There is a lot of code in production that does not earn dollars though, that is not a 1 to 1 measure of success or profitability.
dacydergoth@reddit
If it doesn't follow standards then the linters should catch it. If the linters don't catch it, it can be fixed in a tech debt ticket.
This approach stops a lot of nitpicking, and note that I do say a critical issue may be raised to stop the PR, but it should be critial
Echleon@reddit
No, this is still silly. If some code is written poorly then you should fix it as part of the PR. If that poor code leads to a bug in prod and it’s indecipherable or hard to refactor, you’re going to have to waste more time with a bug in prod.
dacydergoth@reddit
I don't call my respected colleagues with more experience than me silly. Please don't be insulting just because you don't have the experience to understand why I say this.
Echleon@reddit
You’re that upset over the word silly lol? Don’t have silly ideas then silly boy.
dacydergoth@reddit
This is just further demonstrating your lack of maturity and experience.
Think about this .. every PR reviewer has a different approach to PR reviews. Most PR reviewers are not even self consistent. PRs are a valuable tool for catching egregious errors but if bad code gets as far as a PR it should already have been caught by linters, or pre-commit checks, or tests.
If all those checks pass the you want to be a gatekeeper because you are being arrogant and you believe you are the sole arbiter of good code. That is rarely true.
Echleon@reddit
Automated tools can only catch so much, and you should know that if you’re so experienced. If some code needs to go to production ASAP to fix a bug, then sure who cares what the code looks like - just fix the issue at hand. But if we’re doing standard development work? Then there’s no reason to not fix it during PR.
dacydergoth@reddit
If code that bad gets to PR stage you have bigger problems with your development process
RegularLeg7020@reddit
Honestly, it's hard to tell who is in the right without looking at the code.
On one hand, there are people like me who care about the code trying to fight of copy paste and duplication pissing everyone off asking for a redo.
On the other hand, there are people whom are just weaponizing code reviews to make people move stuff around to different classses for inflicting or causing them torture. Talking about indentations, how classes should be named unless yoy named your class or variable A and it's not meaningful at all. Or arguing whether an entrypoint should be in past tense and changing it over and over.
ToThePillory@reddit
Sometimes you just have to play the game. This other developer basically owns the codebase, and wrote most of it.
Changing things without running it by him, rightly or wrongly, might piss him off.
Play the game, talk to him about changes, collaborate, seek advice.
Kind_Animator4149@reddit
If you are new and will not be maintaining the code base for a long time, better to listen to the Sr. He may be thinking long term not for that day/feature alone . But its always better to communicate why you have done it the way you did and how will it be maintainable long term
polaroid_kidd@reddit
I thought I had a lead like that. Then I took over his position and realised why he was like that.
The code base is huge and in very good shape overall. Letting the standards drop here and there introduces rot which spreads...
So now I'm that guy
Schedule_Left@reddit
Shut up and listen. Dude is going to be your Lead Engineer soon, unless you're going to take that crown away from him somehow.
spookydookie@reddit
Can we merge him with my seniors who just rubber stamp PRs without reading them?
Relevant_Mortgage349@reddit (OP)
I don’t think they will be friends lol
Grouchy_Sun_@reddit
So you’re saying there would be a merge conflict?
I’ll see myself out
According_Lab_6907@reddit
you won.
michaelbelgium@reddit
Prrhaps its mentality of "If it works, don't fix it"
But bit ballsy to just get hired and give vibes like "look, my code is better than yours". Normally you communicate and give arguments to why your implemention is better, thats what a team is for
RGBrewskies@reddit
high coding standards are important, you should have high coding standards too.
messy code = buggy code = downtime = its christmas ever and youre sitting at a card table in your uncles basement because prod went down
SelfEnergy@reddit
prod won't go down due to an iframe though
Code-Katana@reddit
Laughs in legacy Perl CGI that brought down prod.
The payment processing iframe was very important it turns out lol
LossPreventionGuy@reddit
oh you sweet summer child....
Whatdoesthis_do@reddit
Same boat here, got a senior as well who wants the entire code base done like he wants it to be. There is just no discussion possible. He has to verify all pull requests and he denies them if its not the way he wants it, up to personal preference. It takes and costs a lot of energy to work and deal with people like that.
maseephus@reddit
High standard good.
Quality post with no examples.
If you’re new, maybe dont try to shake things up right away?
whatsoupman@reddit
I've been in situations like this. It sounds like he doesn't even listen to opinions of others and only wants to do things his way. There are many ways to solve a problem and its tough when your coworker is very opinionated. Agreeing to disagree is a solution but it could impact your morale. Maybe next time communicate before writing code how you will solve the problem and come to an agreement. Another option is to bringing it up to the manager and set expectations and best practices. He has seniority so I would tread carefully. Best of luck.
sobrietyincorporated@reddit
"Disagree then commit."
Meaning you should say why you did or want to do a thing, then accept the answer regardless.
Herrowgayboi@reddit
Same situation I'm dealing with. This one senior dev has been with the company for over 16 years, and talking to everyone else on the team, they always feel it's difficult to work with him just because it's "my way or my way". Nothing is up for discussion. Mainly because he was there since day zero, and everyone else has either gotten laid off, re-org'd, or left somewhere else.
Personally for me, I will disagree with him and stand my ground. Not saying I have the best way out there, but I tend to fallback on "best practices" (which I generally follow), to make it sound less opinionated. With that, I've helped the rest of the team feel empowered to disagree against this senior and go with what they think is best. Obviously, I'm more than willing to help them push forward too if I think they are in the right as well.
RocCityBitch@reddit
How long has it been since you last joined a new team (before this one)? I ask because if it’s been a while, it might be helpful to remind yourself that almost every time a new team is forming (or an existing team gets shaken up enough with new members or restructuring), there’s a transition period where there will be some friction until you settle into a flow as a group.
There’s more than a little truth to the saying for how a team typically progresses: Forming —> Storming —> Performing
How long that second phase lasts comes down to the maturity and experience level of the team, and effective leadership. Have some patience and really try to understand your teammates’ mentality and context in the codebase before feeling it’s appropriate to dig your heels in on opinions. I just get the impression it’s too early.
ewhim@reddit
https://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/
Senior Dev needs to chill out
t1mmen@reddit
Equivalent_Form_9717@reddit
Think you need to communicate with your senior to delve into these issues as your first point. This is your team’s codebase, it’s not yours, so you need to agree on a coding style, and set up a linter to conform to a single style and linting rules.
Seniors who are often nitpicky are commonly right except for special circumstances so I’m giving the benefit of the doubt to your senior on these reviews.