Spent an hour today renaming env vars across three services to make them “consistent.” Broke staging in the process because one service cached the old values. Should’ve just left the mess alone — it worked fine before I touched it.
This is one of those things that break two golden rules. Minimal change, or consistency? The only further variable a most senior person considers is the brittleness of the environment and deployment process.
I feel like it depends on the project.
Typically in the ones I work on, whenever I spot an inconsistency I fix it no matter how much of a mess that causes. Yes it hurts at first, but it may hurt more down the line when the weirdnesses accumulate to the point where it impacts operations and it’ll be a much bigger problem to fix.
However, I can totally see a more chill strategy work better for a project that is mostly just relatively simple code, like perhaps some website deployments. In my opinion it depends a lot on whether the code is generally already complicated, which is when you’ll typically want to refactor earlier than later.
“Let me just clean this thing aaaaaand it’s gone”
I had a senior early on in my career who took pride in producing the smallest diff possible to solve any given ticket and I thought it was so strange – I’m already in the file, why not clean up a bit? In hindsight, they were entirely right, and I always regret doing just a little cleanup without it being strictly necessary.
You can have both. Just not in one MR. Do what is needed, push, branch off, do what is nice
Not if your team’s policy is that every MR must be associated with a Jira ticket for a bug/feature and creating tickets for refactoring/cleanups is simply not done.
I just tag it with the same ticket I was working on when I discovered it needed cleanup. My logic is, it IS directly related to that ticket because that’s when the file was being touched, which implies it’s time is due to clean up some of its technical debt. Nobody’s ever challenged it and I see other people doing similar things. It depends on the company culture, obviously, but it’s not necessarily a no-go.
I’ve seen that before. It’s a self-destructive policy.
Just make the ticket. Be the change you want to see.
Or, as suggested, just attach it to the ticket you were working on like it’s an omnibus.
Agreed.
I’m also a fan of Kent Beck’s “make the change easy, then make the easy change”.
i.e. Do one PR that changes the code’s design but not its behavior, in order to make the next feature easier to implement. Then do another PR that changes just the behavior but not the design.
This way, you get earlier feedback on refactors and you keep behavior PRs small so you reduce the likelihood of the scenario where “we really need to ship this feature but it’s tied to this massive refactor that isn’t quite right”.
(Note that this is just for the case where a feature genuinely requires a refactor. If it’s just “boy-scouting”, then order shouldn’t matter but you should still keep them separate.)
I’m so, so guilty of this. But that’s also because I’ve inherited a codebase that’s almost comically badly structured and every file I open is a new outrage.
Better to break things in staging than prod.
Yeah, that’s the silver lining I guess. Still felt dumb staring at broken builds because of a rename that nobody asked for.
Yes, guilty. But also no longer regretting it a year later now that there’s a clear naming scheme and structure and I can easily find what I’m looking for.
When refactoring, it always gets worse before it gets better. So, make sure to finish it so you get to the “better” state and don’t leave it at “worse”
If you are doing a rename you should have it look for the new environment variable and then fall back to the old one if it’s not set. Log an error when that occurs, and then slowly go through and remove the old environment variables in your deploy code, then remove the compatibility code
The problem with renaming environment variables is that you don’t always control the environment.
- $variable1
- $othervariable
- $july2014fixvariable
- $notneeded
…and finally…
*$otherothervariable
These shall be the variable names until the heat death of the universe! Blessed be! So say we all!
I’ve not done this specifically, but I know the pain of wanting to make things neat and it breaking things. Then having to live with the unsatisfying setup instead of making everything symmetrical and scratching my brain itch.
“If it aint broke don’t fix it”
“If it ain’t broke, it’s about to be.”
At a place years ago, I misspelled a word (stupid English) which made its way into the APIs our FE consumed. FE guy decided to do a quick find/replace and broke prod. He missed some files.
(One of oldest database tables also had a spelling error from the site’s first dev because English sucks)
Edit: WTAF, autocorrect?
I once worked on a project where a third party-parser had a bug that effectively made it sensitive to the order it loaded files.
This wasn’t a syntactic or a semantic problem. There was a stack processing issue that could be avoided by simply loading the files in a different order. This might have changed the final semantics if one file contradicted another, but that generally didn’t happen. Each file had a specific purpose and it would still break without any contradictions.
The default load order was whatever order the file system had the files stored, but knowing that wouldn’t necessarily help because changing the size or contents of a file in such a way that it retained the same position might trigger the same issue. Therefore you could force by-Unicode load ordering and still have it choke.
It was very hard to track down and I very much did not like editing those files for fear of triggering the parser bug. When I had to would be where the regret kicks in.
The other regret, and maybe a little more relevant, was renaming all the files to a different filename format that didn’t make any difference in the end, but it was uglier than the original and it stuck.
I believe a later version of that parser fixed the bug(s), but I moved on.
