Hacker News Reader

> I've seen this same resistance to deleting code in programmers working in repos that are completely under VCS, and it's flabbergasting

I think a lot of developers only know the basics of git. They can check in changes, they can look at history with git log, and maybe they know how to use git blame.

They often don't know how to filter git history. They often don't know about the git pickaxe, or about exclude patterns, and don't even think to question if you can do something like "git log -G'int.*foo\(' -- ':(exclude)directory'" to search for 'foo' in the git log, excluding some directory.

They know how to "grep" within the existing code tree though, so they know if it's not deleted they can find it again with the right grep. If it's deleted, they might not know how to find it in git history.

I sympathize with this to a degree actually. Code in the git log is invisible to a lot of tooling, so for example it won't show up in autocomplete if your text editor might have otherwise suggested it, it won't show up in your library documentation, etc.

If you truly think the code will be used again, I think it's at least defensible to leave it in the tree so that it comes along for refactors, and ends up being found when it's needed.

For cases like Knight capital, where it's obviously never going to be useful again, it's not defensible of course.

I can understand that argument for small, standalone functions. Where it really gets at me is when people insist on leaving whole use cases or subsystems in place, which seems to be what happened here.

You don't want these to be visible to autocomplete, because they're outdated and would need major modification to be correct again. If you do need to resurrect them, they're trivial to find in the git history—just search for the commit named "remove foo"—and they should pass through code review as if they were brand new code, because a lot of stuff will have changed around them in the intervening time.

> git pickaxe, or about exclude patterns, and don't even think to question if you can do something like "git log -G'int.*foo\(' -- ':(exclude)directory'" to search for 'foo' in the git log, excluding some directory

Learned something new today, thank you! Will find ways to use these in my daily workflow.

> and don't even think to question if you can do something like "git log -G'int.*foo\(' -- ':(exclude)directory'"

I do question it, but I know the answer is hard (as you demonstrated), so I don't bother. And I'm now looking at git log docs - I fail to parse how the exclude works even looking at the docs - I don't find anything about `:(` contstruct which houses exclude keyword. But thanks for -G - that will be useful.

> Code in the git log is invisible to a lot of tooling

This is the issue. I expect when searching code to have a way to search older commits. But azure devops won't do it. There is no checkbox "Include all commits"

> I fail to parse how the exclude works even looking at the docs

Most git subcommands can take a pathspec, including git log.

The documentation you want to read is the pathspec docs, see here: https://git-scm.com/docs/gitglossary#Documentation/gitglossa...

You can see that it includes the exclude keyword, among others.

Since it applies to almost every command (from 'git add -- <pathspec>' to 'git checkout -- <pathspec>'), it's not mentioned as clearly in individual commands.

git log and all other commands' man page should really refer back to gitglossary here then. And they should either name their argument <pathspec>, or specify that the given argument (<path> or <file>) is a pathspec.

"If it works, don't touch it" is something I've heard a lot, especially said by managers who don't understand what they are talking about.

An update to simply "remove old code" might be difficult if someone sees any change as creating a risk of something going wrong. And to be fair, any change is a risk, but so is leaving old code around.

At least now we have this case to point to as a clear example of the risk.

I'm less surprised by them leaving dead code around, honestly. I've seen that happen at virtually every company I've worked for.

What completely baffles me everytime I come across their story is that they repurposed an existing flag instead of making a new one! Why?

Version control isn’t bulletproof either. All your code history is one git rebase away from being abolished forever.

I hope most orgs have processes around their main branches so this does not occur, but I’ve also been in smaller orgs and accidentally screwed up prod database tables, so the accidental git rebase isn’t impossible to consider…

> All your code history is one git rebase away from being abolished forever

unless you also GC right away, it's not gone.

Yep, and also:

1. You shouldn't be allowing anybody to force-push rebased stuff onto major branches in your main repo (the one builds come from) anyway. This is especially important to support auditing and trace-ability.

2. Just because the folder is a git-repo isn't an excuse not to have it part of your regular offsite backup set.

There are reasons to want branch protection but audit isn’t one of them. An auditable system would be keeping an immutable log of the git repo actions in a separate, append-only location. It wouldn’t rely on the thing being written to all the time, by users, to never get broken. In your model, your audit history only needs one bug in the branch-protection tool for it to be destroyed.

Think of it like syslog. It’s good to keep a log of events but it’s bad to rely on the /var/log/syslog on your web server. You should be logging to a remote, append-only system.

(However I would concede that if we are talking about lawyer-proof-audit — SOC2, ISO etc — rather than actual security auditing, then branch protection is probably just fine.)

Yes, you should definitely have branch protection turned on on main, I kind of assumed that went without saying. But to actually lose your entire git history would require both having no branch protection and having every single developer on your staff be in the regular habit of force rebasing their own branches on main. If a single developer does a double take when they're told that their branch has a different history than origin/main, then you probably didn't lose more than a week of work.

Also, it's worth noting that this dead code will almost certainly not get reused wherever you decide to store it, so it's best to keep it as far away from zombification as possible, even if it's not the safest place.

You're very unlikely to lose your version history like that.

Everywhere I've worked has branch protection turned on, and backups. Even if those both fail somehow it's very likely the complete history is on lots of engineers' laptops.

See my sister thread where I worked at Knight just after the outage.

> For 8 years they left unused code in place, seemingly only bothering to remove it because they wanted to repurpose a flag

There was another issue where they were using a database with only 256 columns. Sometimes, they needed a new column so they would just reuse an old columns that "wasn't being used at the time".

IIRC, this was generally acknowledged internally to be "a bad idea" but no one had prioritized cleaning up the old code and/or coming up with a better best practice.

Maybe they had engineering leadership mandates where creating new feature flags was more paperwork?

I've seen that happen, where 'new' features took a lot of justification, but 'bugfixes' were free.

Imo it's easier to quantify the value of adding new code than removing old code (in terms of dollars). Things that are more easily to quantify the value of tend to get higher priority. Same applies to other areas like performance optimization, testing, security, patching (some of these have high level data like the cost of incidents/downtime or the cost of a breach)

Some companies have processes in place to try to balance this like 20% of time spent on tech debt reduction

I always remember the Codeless Code on comments when I read about this: http://thecodelesscode.com/case/41?topic=comments

nitpick: git is enough to keep the history of old code, but it's not great at letting you find it.

The morale should be "don't reuse feature flags kids".