20 ms·

I agree that consistency is important — but what about when the existing codebase is already inconsistent? Even worse, what if the existing codebase is both inc

I agree that consistency is important — but what about when the existing codebase is already inconsistent? Even worse, what if the existing codebase is both inconsistent and the "right way to do things" is undocumented? That's much closer to what I've experienced when joining companies with lots of existing code. In this scenario, I've found that the only productive way forward is to do the best job you can, in your own isolated code, and share loudly and frequently why you're doing things your new different way. Write your code to be re-used and shared. Write docs for it. Explain why it's the correct approach. Ask for feedback from the wider engineering org (although don't block on it if they're not directly involved with your work.) You'll quickly find out if other engineers agree that your approach is better. If it's actually better, others will start following your lead. If it's not, you'll be able to adjust. Of course, when working in the existing code, try to be as locally consistent as possible with the surrounding code, even if it's terrible. I like to think of this as "getting in and out" as quickly as possible. If you encounter particularly sticky/unhelpful/reticent team members, it can help to remind them that (a) the existing code is worse than what you're writing, (b) there is no documented pattern that you're breaking, (c) your work is an experiment and you will later revise it. Often asking them to simply document the convention that you are supposedly breaking is enough to get them to go away, since they won't bother to spend the effort.

deleted 2y ago
[deleted]
mgfist 2y ago
I rarely see large 10m+ LOC codebases with any sort of strong consistency. There are always flavors of implementations and patterns all over the place. Hell, it's common to see some functionality implemented multiple times in different places
- peterldowns 2y ago
  And it's fine, right? Honestly I think people need to realize that part of being a good engineer is being able to deal with inconsistency. Maybe submodule A and submodule B do network requests slightly differently but if both ways are reasonable, working, and making the company money, it's probably not worth delaying product improvements in order to make things "more consistent." On the other hand if no one in your company cares about consistency, at some point everything becomes so awful you basically won't be able to retain engineers or hire new ones, so this is a place where careful judgement is needed.
  - dasil003 2y ago
    Yeah 100%. Honestly style / technique / language consistency are implementation details, it helps with engineer fungibility and ramp up, but it also works against engineers applying local judgement. This is something to briefly consider when starting new services/features, but definitely not something to optimize for in an existing system. On the other hand, data and logic consistency can be really important, but you still have to pick your battles because it's all tradeoffs. I've done a lot of work in pricing over the decades, and it tends to be an area where the logic is complex and you need consistency across surfaces owned by many teams, but at the same time it will interact with local features that you don't want to turn pricing libraries/services into god objects as you start bottlenecking all kinds of tangentially related projects. It's a very tricky balance to get right. My general rule of thumb is to anchor on user impact as the first order consideration, developer experience is important as a second order, but many engineers will over-index on things they are deeply familiar with and not be objective in their evaluation of the impact / cost to other teams who pay an interaction cost but are not experts in the domain.
    - Supermancho 2y ago
      A common experience (mostly in the Pacific North West) I have had is to implement a feature in a straightforward manner that works with minimal code, for some backlog issue. Then I'm told the PR will be looked at. A couple days later I am told this is not the way to do X. You must do it Y? Why Y? Because of historical battles won and lost why, not because of a specific characteristic. My PR doesn't work with Y and it would be more complicated...like who knows what multiplier of code to make it work. Well that makes it a harder task than your estimate, which is why nobody ever took it up before and was really excited about your low estimate. How does Y work? Well it works specifically to prevent features like X. How am I supposed to know how to modify Y in a way that satisfies the invisible soft requirements? Someone more senior takes over my ticket, while I'm assigned unit tests. They end up writing a few hundred lines of code for Y2.0 then implement X with a copy paste of a few lines. I must not be "a good fit". Welcome to the next 6-12 months of not caring about this job at all, while I find another job without my resume starting to look like patchwork. Challenging people's egos by providing a simpler implementation for something someone says is very hard, has been effective at getting old stagnant issues completed. Unnaturally effective. Of course, those new "right way" features are just as ugly as any existing feature, ensuring the perpetuation of the code complexity. Continually writing themselves into corners they don't want to mess with.
- wrs 2y ago
  And to be practical, that's fine. In a big codebase it's more important to encourage consistent, well-defined, small interfaces, and a clean separation of concerns, than to try to get consistency in the lower-level implementation details. Other non-code concerns like coordinating releases and migration of shared services are also way more important than getting everyone to use the same string library. (Of course, if you carry that principle to the extreme you end up with a lot of black-box networked microservices.)
digging 2y ago
> If it's actually better, others will start following your lead. Not really my experience in teams that create inconsistent, undocumented codebases... but you might get 1 or 2 converts.
- peterldowns 2y ago
  It depends on the day but generally I believe that most engineers want to write good code, want to improve their own skills, and like learning and critiquing with other engineers. Sometimes a small catalyst is all it takes to dramatically improve things. Most of the times I've thought that individual contributors were the problem, the real issue was what the company's leaders were punishing/rewarding/demanding.
  - darepublic 2y ago
    doing some recent contract work I discovered someone putting this into a PR (comments my own) ``` let susJsonString = '...' // we get this parseable json string from somwhere but of course it might not be parseable. so testing seems warranted... try { // lets bust out a while loop! while(typeof susJsonString === 'string') { susJsonString = JSON.parse(susJsonString) } } catch { susJsonString = {} } // also this was a typescript codebase but all the more reason to have a variable switch types! this dev undoubtedly puts typescript at the top of their resume ``` I suppose this works?! I haven't thought it through carefully, it's just deciding to put your shoes on backward, and open doors while standing on your head. But I decided to just keep out of it, not get involved in the politics. I guess this is what getting old is like seriously you just see younger people doing stuff that makes your jaw drop from the stupidity (or maybe its just me) but you can't say anything because reasons. Copilot, ai assisted coding only further muddies the waters imo.
    - liontwist 2y ago
      This is totally fine. If you're given shit data this seems like a reasonable way to try to parse it (I would personally bound the loop). Typescript is not going to make it better. The problem is whoever is producing the data.
HideousKojima 2y ago
My approach is what I call defensive programming, with a different meaning than the usual usage of the term. I assume that my coworkers are idiots that aren't going to read my documentation, so I make all public classes and methods etc. as idiot-proof as possible to use. Hasn't saved me from every issue caused by my teammates never reading my docs or asking me questions, but it's definitely prevented several.
- staunton 2y ago
  > assume that my coworkers are idiots I know (most?) people don't mean it literally when writing something like this but I still wonder why such self-evident ideas as "make things easy to use correctly and hard to use incorrectly" are framed in terms of "idiots who don't rtfm". The best documentation is what wasn't written because it (actually!) wasn't needed. On the other hand, even if people aren't "idiots", they still make mistakes and take time to figure out (perhaps by reading tfm) how to do things and complete their tasks, all of which has a cost. Making this easier is a clear benefit.
  - s4i 2y ago
    The bigger the codebase, we all eventually find ourselves in scenarios where we were the idiot. Making the APIs as foolproof as possible, utilizing tools like Semgrep, deprecating stuff in the way your language supports that it shows up in the IDE,… all that stuff should be utilized.
agentultra 2y ago
> In this scenario, I've found that the only productive way forward is to do the best job you can, in your own isolated code, and share loudly and frequently why you're doing things your new different way. Now you have N+1 ways. It can work if you manage to get a majority of a team to support your efforts, create good interfaces into the legacy code paths, and most importantly: write meaningful and useful integration tests against that interface. Michael Feathers wrote a wonderful book about this called, Working Effectively with Legacy Code. I think what the author is trying to say with consistency is to avoid adding even more paths, layers, and indirection in an already untested and difficult code base. Work strategically, methodically, and communicate well as you say and it can be a real source of progress with an existing system.
- peterldowns 2y ago
  I’ll check out that book, thanks for the reference.
darepublic 2y ago
My last experience with this, in the section of code I had to navigate for my first ticket at startup X we had some code that was querying the same tables multiple times unnecessarily. We were also using a highly bespoke (imo) code library with a relatively small following on github but this library permeated the entire codebase and dictated the way things had to be done. I tried to just make my changes by touching the code as little as possible, but avoiding the most outstanding inefficiencies. I thought of my changes as a small oasis of sanity in a desert of madness. In that first PR the seniors tore me a new one. It turned out there was a v2 of "the right way of doing things" that I had to learn about and then write my code to conform to. v2 had it's own issues, though was perhaps not as bad as v1. Later on when I became more influential I was able to successfully advocate to management to change many things to my liking, including axing the beloved exotic library that distinguished our codebase. But the old guard remained highly resistant to changing the way our code was written, and switched their stance from 'this is great' to, 'it sucks but its too much effort not to keep with it'. I am left feeling that it was all not worth it, not just the struggle but whether the product was appreciably effected one way or another. Just another crappy war story of my blighted career.
- dirtbag__dad 2y ago
  This resonates hard. In particular, managing the old guard’s emotions is a defeating process. It is almost always easier to jump into a project and critique it than it is to start from scratch. New perspectives and better ideas should be welcome, but instead they can be shut down because folks take things personally. My (rather unfortunate) conclusion is that when I encounter this behavior I move to another team to avoid it. If that’s not possible it’s honestly worth looking for another job.
- physicsguy 2y ago
  I had this in 2018 with a company that were still using csh scripts in all of the development tooling.
lamontcg 2y ago
It is terrible to just do this on your own, particularly as the n00b. If there are 5 different standards in the codebase, don't just invent your own better way of doing things. That is literally the xkcd/Standards problem. Go find one of the people who have worked there the longest and ask which of the 5 existing standards are most modern and should be copied. And as you get more experience with the codebase you can suggest updates to the best standard and evolve it. The problem is that you then need to own updating that whole standard across the entire codebase. That's the hard part. If you aren't experienced enough with the codebase to be aggressive about standardization, you shouldn't be creating some little playground of your own.
- peterldowns 2y ago
  > If there are 5 different standards in the codebase, don't just invent your own better way of doing things. That is literally the xkcd/Standards problem. Go find one of the people who have worked there the longest and ask which of the 5 existing standards are most modern and should be copied. I strongly disagree with you and believe you've missed the point of my comment. Think about this: why are there 5 different standards in the codebase, none of which meet your needs? Do you think any engineers on the team are aware of this situation? And how might you get more experience with the codebase without writing code that solves your problems?
  - jiggawatts 2y ago
    “Time” is the answer almost always. Standards evolve over time, as do the languages and frameworks. Old code is rarely rewritten, so you end up with layers of code like geological strata recording the history of the developer landscape. There’s a more complicated aspect of “Conway’s law, but over time” that’s hard to explain in a comment. And anyway, Casey Muratori did it better: https://youtu.be/5IUj1EZwpJY?si=hnrKXeknMCe0UPv4 https://youtu.be/5IUj1EZwpJY?si=hnrKXeknMCe0UPv4
  - baq 2y ago
    People who created 3 of the 5 no longer work at the company, test coverage is minimal or nonexistent and the system mostly does what it’s supposed to. In this situation, ‘getting more experience in the code base’ is more or less synonymous with ‘getting paged on the weekend’.
sfink 2y ago
Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms. In that case, if the current way is undocumented and/or inconsistent, you make it better before or while adding in your new approach. If there are 4 ways to do the same thing and you really want to do it a different way, then replace one of those ways with your new one in the process of adding it. For extra credit, get to the point where you understand why you can't replace the other 3. (Or if you can, do it! Preferably in followups, to avoid bundling too much risk at once.) A lot of inconsistency is the result of unwillingness to fix other people's stuff. If your way is better, trust people to see it when applied to their own code. They probably have similar grievances, but it has never been a priority to fix. If you're willing to spend the time and energy, there's a good chance they'll be willing to accept the results even if it does cause some churn and require new learning. (Source: I have worked on Firefox for a decade now, which fits the criteria in the article, and sweeping changes that affect the entire codebase are relatively common. People here are more likely to encourage such thinking than to shoot it down because it is new or different than the status quo. You just can't be an ass about it and ignore legitimate objections. It is still a giant legacy codebase with ancient warts, but I mostly see technical or logistical obstacles to cleaning things up, not political ones.)
- peterldowns 2y ago
  I don't disagree with what you've written at all, but let me just say: > Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms ah to be so lucky...
  - sverhagen 2y ago
    It's a bit of a non-issue in this context. If you don't have a mono-repo, you should maintain reasonable consistency within each repository (and hope they're consistent between each other, but that's probably less important here).
- StellarScience 2y ago
  Great points, I'd just add: > A lot of inconsistency is the result of unwillingness to fix other people's stuff Agree, so we find it best to practice "no code ownership" or better yet "shared code ownership." So we try to think of it all as "our stuff" rather than "other people's stuff." Maybe you just joined the project, and are working around code that hasn't been touched in 5 years, but we're all responsible for improving the code and making it better as we go. That requires a high trust environment; I don't know if it could work for Firefox where you may have some very part-time contributors. But having documented standards, plus clang-format and clang-tidy to automate some of the simpler things, also goes a long way.
stravant 2y ago
Sometimes people are too afraid of attempting to make it consistent. I've done several migrations of thing with dozens of unique bespoke usage patterns back to a nice consistent approach. It sometimes takes a couple straight days of just raw focused code munging, and doesn't always end up being viable, but it's worth a shot for how much better a state it can leave things in.
- sfn42 2y ago
  Highly agree. I've done quite a few large refactors of unnecessarily complex systems that resulted in significant improvement - from lots of bugs to nearly no bugs, incomprehensible code to simple straight forward code, no tests to great test coverage. I did have one bad experience where I ended up spending way too much time on a project like that, I think I made some mistakes with that one and got in a bit too deep. Luckily my team was very supportive and I was able to finish it and it's a lot better now than it was.
bandrami 2y ago
"Now we have five inconsistent coding conventions..."
whilenot-dev 2y ago
Consistency is never the sole reason to change something, there's always consistency and at least one of the following: - coherency - type safety (shout-out to dynamically typed languages that adapted static typing) - concurrency - simplicity - (and more) > If it's actually better, others will start following your lead. A lot of people don't want to improve the quality in their output and for various reasons... some are happy to have something "to pay the bills", some don't want to use a programming language to its full extend, some have a deeply rooted paradigm that worked for 10 years already ("static types won't change that"), others are scared of concurrency etc. For some people there's nothing to worry about when a server can be blocked by a single request for 60 secs.
virgilp 2y ago
Relevant xkcd: https://xkcd.com/927/ https://xkcd.com/927/
DrBazza 2y ago
> but what about when the existing codebase is already inconsistent It depends. If it's a self contained code base, automatic refactoring can safely and mechanically update code to be consistent (naming, and in some cases structurally). If it's not self contained and you're shipping libraries, i.e. things depend on your code base, then it's more tricky, but not impossible. "Lean on tooling" is the first thing you should do.
mihaaly 2y ago
Consistency in huge legacy codebase is like virginity in a brothel: desired but non-existent.
nuancebydefault 2y ago
To me the (a), (b) and (c) tactics remind me of people who were very hard to work with. I believe a better approach is indeed like you already mentioned, explain and document, but, as an extra: also be open to comments on the docs and implementation. Often there's a reason that your particular approach was not used earlier, like explained in the article.
LouisSayers 2y ago
> but what about when the existing codebase is already inconsistent? Then you get people together to agree what consistent looks like. I find the easiest way to do this is to borrow someone else's publicly documented coding conventions e.g. Company ABC. Then anyone disagreeing isn't disagreeing with you, they're disagreeing with Company ABC, and they (and you) just have to suck it up. From there on in, you add linting tools, PR checks etc for any new code that comes in.
- cess11 2y ago
  If there's resistance to picking a style guide, autoformatting might be a viable start and will probably do quite a bit for shallow consistency at the price of large PR:s once per file. Once one has worked with a forced style for a while it starts to feel weird to see breaches of it, and I think that might help softening people to adapting a style guide regarding more subtle things like error handling or attitude to standardised protocols like HTTP.
dotancohen 2y ago
> your work is an experiment and you will later revise it I advise against this if you have not been allocated the time or budget to revise the code. For one thing, you're lying. For another thing, were you hired to be a part of the contributing team or hired to be part of a research team doing experiments in the contributing team's codebase and possibly deploying your experiment on their production systems? I would immediately push back on any new guy who says this, no matter how confident he seems that his way is the right way.
- mihaaly 2y ago
  Counter-thought: We are making brand new things here and not being in an assembly line coming up with the very same thing dozens to million times. We are paid to make new products never existed, having novelty elements in it desired to be a bigger extent than not! Those pretending knowing exactly what they are doing are lying! Of course we are speculating here about the size of novelty content to a differing extent, which is never 0% and never 100%, but something inbetween. But those pushing back on those at least trying to revise the work - putting emphasis on it -, deserve no-one coming to them to be pushed back (at least for the inability of allocating resources for this essential activity of development. Development!). (tried to mimic the atmosphere of the message, sorry if failed)

deleted 2y ago

[deleted]

mgfist 2y ago

I rarely see large 10m+ LOC codebases with any sort of strong consistency. There are always flavors of implementations and patterns all over the place. Hell, it's common to see some functionality implemented multiple times in different places

peterldowns 2y ago
And it's fine, right? Honestly I think people need to realize that part of being a good engineer is being able to deal with inconsistency. Maybe submodule A and submodule B do network requests slightly differently but if both ways are reasonable, working, and making the company money, it's probably not worth delaying product improvements in order to make things "more consistent." On the other hand if no one in your company cares about consistency, at some point everything becomes so awful you basically won't be able to retain engineers or hire new ones, so this is a place where careful judgement is needed.
- dasil003 2y ago
  Yeah 100%. Honestly style / technique / language consistency are implementation details, it helps with engineer fungibility and ramp up, but it also works against engineers applying local judgement. This is something to briefly consider when starting new services/features, but definitely not something to optimize for in an existing system. On the other hand, data and logic consistency can be really important, but you still have to pick your battles because it's all tradeoffs. I've done a lot of work in pricing over the decades, and it tends to be an area where the logic is complex and you need consistency across surfaces owned by many teams, but at the same time it will interact with local features that you don't want to turn pricing libraries/services into god objects as you start bottlenecking all kinds of tangentially related projects. It's a very tricky balance to get right. My general rule of thumb is to anchor on user impact as the first order consideration, developer experience is important as a second order, but many engineers will over-index on things they are deeply familiar with and not be objective in their evaluation of the impact / cost to other teams who pay an interaction cost but are not experts in the domain.
  - Supermancho 2y ago
    A common experience (mostly in the Pacific North West) I have had is to implement a feature in a straightforward manner that works with minimal code, for some backlog issue. Then I'm told the PR will be looked at. A couple days later I am told this is not the way to do X. You must do it Y? Why Y? Because of historical battles won and lost why, not because of a specific characteristic. My PR doesn't work with Y and it would be more complicated...like who knows what multiplier of code to make it work. Well that makes it a harder task than your estimate, which is why nobody ever took it up before and was really excited about your low estimate. How does Y work? Well it works specifically to prevent features like X. How am I supposed to know how to modify Y in a way that satisfies the invisible soft requirements? Someone more senior takes over my ticket, while I'm assigned unit tests. They end up writing a few hundred lines of code for Y2.0 then implement X with a copy paste of a few lines. I must not be "a good fit". Welcome to the next 6-12 months of not caring about this job at all, while I find another job without my resume starting to look like patchwork. Challenging people's egos by providing a simpler implementation for something someone says is very hard, has been effective at getting old stagnant issues completed. Unnaturally effective. Of course, those new "right way" features are just as ugly as any existing feature, ensuring the perpetuation of the code complexity. Continually writing themselves into corners they don't want to mess with.
wrs 2y ago
And to be practical, that's fine. In a big codebase it's more important to encourage consistent, well-defined, small interfaces, and a clean separation of concerns, than to try to get consistency in the lower-level implementation details. Other non-code concerns like coordinating releases and migration of shared services are also way more important than getting everyone to use the same string library. (Of course, if you carry that principle to the extreme you end up with a lot of black-box networked microservices.)

digging 2y ago

> If it's actually better, others will start following your lead. Not really my experience in teams that create inconsistent, undocumented codebases... but you might get 1 or 2 converts.

peterldowns 2y ago
It depends on the day but generally I believe that most engineers want to write good code, want to improve their own skills, and like learning and critiquing with other engineers. Sometimes a small catalyst is all it takes to dramatically improve things. Most of the times I've thought that individual contributors were the problem, the real issue was what the company's leaders were punishing/rewarding/demanding.
- darepublic 2y ago
  doing some recent contract work I discovered someone putting this into a PR (comments my own) ``` let susJsonString = '...' // we get this parseable json string from somwhere but of course it might not be parseable. so testing seems warranted... try { // lets bust out a while loop! while(typeof susJsonString === 'string') { susJsonString = JSON.parse(susJsonString) } } catch { susJsonString = {} } // also this was a typescript codebase but all the more reason to have a variable switch types! this dev undoubtedly puts typescript at the top of their resume ``` I suppose this works?! I haven't thought it through carefully, it's just deciding to put your shoes on backward, and open doors while standing on your head. But I decided to just keep out of it, not get involved in the politics. I guess this is what getting old is like seriously you just see younger people doing stuff that makes your jaw drop from the stupidity (or maybe its just me) but you can't say anything because reasons. Copilot, ai assisted coding only further muddies the waters imo.
  - liontwist 2y ago
    This is totally fine. If you're given shit data this seems like a reasonable way to try to parse it (I would personally bound the loop). Typescript is not going to make it better. The problem is whoever is producing the data.

HideousKojima 2y ago

My approach is what I call defensive programming, with a different meaning than the usual usage of the term. I assume that my coworkers are idiots that aren't going to read my documentation, so I make all public classes and methods etc. as idiot-proof as possible to use. Hasn't saved me from every issue caused by my teammates never reading my docs or asking me questions, but it's definitely prevented several.

staunton 2y ago
> assume that my coworkers are idiots I know (most?) people don't mean it literally when writing something like this but I still wonder why such self-evident ideas as "make things easy to use correctly and hard to use incorrectly" are framed in terms of "idiots who don't rtfm". The best documentation is what wasn't written because it (actually!) wasn't needed. On the other hand, even if people aren't "idiots", they still make mistakes and take time to figure out (perhaps by reading tfm) how to do things and complete their tasks, all of which has a cost. Making this easier is a clear benefit.
- s4i 2y ago
  The bigger the codebase, we all eventually find ourselves in scenarios where we were the idiot. Making the APIs as foolproof as possible, utilizing tools like Semgrep, deprecating stuff in the way your language supports that it shows up in the IDE,… all that stuff should be utilized.

agentultra 2y ago

> In this scenario, I've found that the only productive way forward is to do the best job you can, in your own isolated code, and share loudly and frequently why you're doing things your new different way. Now you have N+1 ways. It can work if you manage to get a majority of a team to support your efforts, create good interfaces into the legacy code paths, and most importantly: write meaningful and useful integration tests against that interface. Michael Feathers wrote a wonderful book about this called, Working Effectively with Legacy Code. I think what the author is trying to say with consistency is to avoid adding even more paths, layers, and indirection in an already untested and difficult code base. Work strategically, methodically, and communicate well as you say and it can be a real source of progress with an existing system.

peterldowns 2y ago
I’ll check out that book, thanks for the reference.

darepublic 2y ago

My last experience with this, in the section of code I had to navigate for my first ticket at startup X we had some code that was querying the same tables multiple times unnecessarily. We were also using a highly bespoke (imo) code library with a relatively small following on github but this library permeated the entire codebase and dictated the way things had to be done. I tried to just make my changes by touching the code as little as possible, but avoiding the most outstanding inefficiencies. I thought of my changes as a small oasis of sanity in a desert of madness. In that first PR the seniors tore me a new one. It turned out there was a v2 of "the right way of doing things" that I had to learn about and then write my code to conform to. v2 had it's own issues, though was perhaps not as bad as v1. Later on when I became more influential I was able to successfully advocate to management to change many things to my liking, including axing the beloved exotic library that distinguished our codebase. But the old guard remained highly resistant to changing the way our code was written, and switched their stance from 'this is great' to, 'it sucks but its too much effort not to keep with it'. I am left feeling that it was all not worth it, not just the struggle but whether the product was appreciably effected one way or another. Just another crappy war story of my blighted career.

dirtbag__dad 2y ago
This resonates hard. In particular, managing the old guard’s emotions is a defeating process. It is almost always easier to jump into a project and critique it than it is to start from scratch. New perspectives and better ideas should be welcome, but instead they can be shut down because folks take things personally. My (rather unfortunate) conclusion is that when I encounter this behavior I move to another team to avoid it. If that’s not possible it’s honestly worth looking for another job.
physicsguy 2y ago
I had this in 2018 with a company that were still using csh scripts in all of the development tooling.

lamontcg 2y ago

It is terrible to just do this on your own, particularly as the n00b. If there are 5 different standards in the codebase, don't just invent your own better way of doing things. That is literally the xkcd/Standards problem. Go find one of the people who have worked there the longest and ask which of the 5 existing standards are most modern and should be copied. And as you get more experience with the codebase you can suggest updates to the best standard and evolve it. The problem is that you then need to own updating that whole standard across the entire codebase. That's the hard part. If you aren't experienced enough with the codebase to be aggressive about standardization, you shouldn't be creating some little playground of your own.

peterldowns 2y ago
> If there are 5 different standards in the codebase, don't just invent your own better way of doing things. That is literally the xkcd/Standards problem. Go find one of the people who have worked there the longest and ask which of the 5 existing standards are most modern and should be copied. I strongly disagree with you and believe you've missed the point of my comment. Think about this: why are there 5 different standards in the codebase, none of which meet your needs? Do you think any engineers on the team are aware of this situation? And how might you get more experience with the codebase without writing code that solves your problems?
- jiggawatts 2y ago
  “Time” is the answer almost always. Standards evolve over time, as do the languages and frameworks. Old code is rarely rewritten, so you end up with layers of code like geological strata recording the history of the developer landscape. There’s a more complicated aspect of “Conway’s law, but over time” that’s hard to explain in a comment. And anyway, Casey Muratori did it better: https://youtu.be/5IUj1EZwpJY?si=hnrKXeknMCe0UPv4 https://youtu.be/5IUj1EZwpJY?si=hnrKXeknMCe0UPv4
- baq 2y ago
  People who created 3 of the 5 no longer work at the company, test coverage is minimal or nonexistent and the system mostly does what it’s supposed to. In this situation, ‘getting more experience in the code base’ is more or less synonymous with ‘getting paged on the weekend’.

sfink 2y ago

Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms. In that case, if the current way is undocumented and/or inconsistent, you make it better before or while adding in your new approach. If there are 4 ways to do the same thing and you really want to do it a different way, then replace one of those ways with your new one in the process of adding it. For extra credit, get to the point where you understand why you can't replace the other 3. (Or if you can, do it! Preferably in followups, to avoid bundling too much risk at once.) A lot of inconsistency is the result of unwillingness to fix other people's stuff. If your way is better, trust people to see it when applied to their own code. They probably have similar grievances, but it has never been a priority to fix. If you're willing to spend the time and energy, there's a good chance they'll be willing to accept the results even if it does cause some churn and require new learning. (Source: I have worked on Firefox for a decade now, which fits the criteria in the article, and sweeping changes that affect the entire codebase are relatively common. People here are more likely to encourage such thinking than to shoot it down because it is new or different than the status quo. You just can't be an ass about it and ignore legitimate objections. It is still a giant legacy codebase with ancient warts, but I mostly see technical or logistical obstacles to cleaning things up, not political ones.)

peterldowns 2y ago
I don't disagree with what you've written at all, but let me just say: > Hopefully, you have a monorepo or something with similar effects, and a lack of fiefdoms ah to be so lucky...
- sverhagen 2y ago
  It's a bit of a non-issue in this context. If you don't have a mono-repo, you should maintain reasonable consistency within each repository (and hope they're consistent between each other, but that's probably less important here).
StellarScience 2y ago
Great points, I'd just add: > A lot of inconsistency is the result of unwillingness to fix other people's stuff Agree, so we find it best to practice "no code ownership" or better yet "shared code ownership." So we try to think of it all as "our stuff" rather than "other people's stuff." Maybe you just joined the project, and are working around code that hasn't been touched in 5 years, but we're all responsible for improving the code and making it better as we go. That requires a high trust environment; I don't know if it could work for Firefox where you may have some very part-time contributors. But having documented standards, plus clang-format and clang-tidy to automate some of the simpler things, also goes a long way.

stravant 2y ago

Sometimes people are too afraid of attempting to make it consistent. I've done several migrations of thing with dozens of unique bespoke usage patterns back to a nice consistent approach. It sometimes takes a couple straight days of just raw focused code munging, and doesn't always end up being viable, but it's worth a shot for how much better a state it can leave things in.

sfn42 2y ago
Highly agree. I've done quite a few large refactors of unnecessarily complex systems that resulted in significant improvement - from lots of bugs to nearly no bugs, incomprehensible code to simple straight forward code, no tests to great test coverage. I did have one bad experience where I ended up spending way too much time on a project like that, I think I made some mistakes with that one and got in a bit too deep. Luckily my team was very supportive and I was able to finish it and it's a lot better now than it was.

bandrami 2y ago

"Now we have five inconsistent coding conventions..."

whilenot-dev 2y ago

Consistency is never the sole reason to change something, there's always consistency and at least one of the following: - coherency - type safety (shout-out to dynamically typed languages that adapted static typing) - concurrency - simplicity - (and more) > If it's actually better, others will start following your lead. A lot of people don't want to improve the quality in their output and for various reasons... some are happy to have something "to pay the bills", some don't want to use a programming language to its full extend, some have a deeply rooted paradigm that worked for 10 years already ("static types won't change that"), others are scared of concurrency etc. For some people there's nothing to worry about when a server can be blocked by a single request for 60 secs.

virgilp 2y ago

Relevant xkcd: https://xkcd.com/927/ https://xkcd.com/927/

DrBazza 2y ago

> but what about when the existing codebase is already inconsistent It depends. If it's a self contained code base, automatic refactoring can safely and mechanically update code to be consistent (naming, and in some cases structurally). If it's not self contained and you're shipping libraries, i.e. things depend on your code base, then it's more tricky, but not impossible. "Lean on tooling" is the first thing you should do.

mihaaly 2y ago

Consistency in huge legacy codebase is like virginity in a brothel: desired but non-existent.

nuancebydefault 2y ago

To me the (a), (b) and (c) tactics remind me of people who were very hard to work with. I believe a better approach is indeed like you already mentioned, explain and document, but, as an extra: also be open to comments on the docs and implementation. Often there's a reason that your particular approach was not used earlier, like explained in the article.

LouisSayers 2y ago

> but what about when the existing codebase is already inconsistent? Then you get people together to agree what consistent looks like. I find the easiest way to do this is to borrow someone else's publicly documented coding conventions e.g. Company ABC. Then anyone disagreeing isn't disagreeing with you, they're disagreeing with Company ABC, and they (and you) just have to suck it up. From there on in, you add linting tools, PR checks etc for any new code that comes in.

cess11 2y ago
If there's resistance to picking a style guide, autoformatting might be a viable start and will probably do quite a bit for shallow consistency at the price of large PR:s once per file. Once one has worked with a forced style for a while it starts to feel weird to see breaches of it, and I think that might help softening people to adapting a style guide regarding more subtle things like error handling or attitude to standardised protocols like HTTP.

dotancohen 2y ago

> your work is an experiment and you will later revise it I advise against this if you have not been allocated the time or budget to revise the code. For one thing, you're lying. For another thing, were you hired to be a part of the contributing team or hired to be part of a research team doing experiments in the contributing team's codebase and possibly deploying your experiment on their production systems? I would immediately push back on any new guy who says this, no matter how confident he seems that his way is the right way.

mihaaly 2y ago
Counter-thought: We are making brand new things here and not being in an assembly line coming up with the very same thing dozens to million times. We are paid to make new products never existed, having novelty elements in it desired to be a bigger extent than not! Those pretending knowing exactly what they are doing are lying! Of course we are speculating here about the size of novelty content to a differing extent, which is never 0% and never 100%, but something inbetween. But those pushing back on those at least trying to revise the work - putting emphasis on it -, deserve no-one coming to them to be pushed back (at least for the inability of allocating resources for this essential activity of development. Development!). (tried to mimic the atmosphere of the message, sorry if failed)