Comment by Jtsummers | Hacker News Reader

Comment by Jtsummers | Hacker News Reader

I'd swap 2 and 3. Getting CI, linting, auto-formatting, etc. going is a higher priority than tearing things out. Why? Because you don't know what to tear out yet or even the consequence of tearing them out. Linting (and other static analysis tools) also give you a lot of insight into where the program needs work.

Things that get flagged by a static analysis tool (today) will often be areas where you can tear out entire functions and maybe even classes and files because they'll be a re-creation of STL concepts. Like homegrown iterator libraries (with subtle problems) that can be replaced with the STL algorithms library, or homegrown smart pointers that can just be replaced with actual smart pointers, or replacing the C string functions with C++'s on string class (and related classes) and functions/methods.

But you won't see that as easily until you start scanning the code. And you won't be able to evaluate the consequences until you push towards more rapid test builds (at least) if not deployment.

Yeah, I've done a fair bit of agency work dropping in to rescue code bases, and the first thing I do is run unit tests and check coverage. I add basic smoke tests anywhere they're missing. This actually speeds me up, rather than slowing me down, because once I have reasonably good coverage I can move dramatically faster when refactoring. It's a small investment that pays off.

On the flip side, auto-formatting will trash your version history and impede analysis of "when and why was this line added".

You can ignore commits from git blame by adding them to a .gitattributes file.

This is assuming Git of course, which is not a given at all for the average legacy c++ codebase.

Good to know. Thanks for the tip!

I'm not hardcore on auto-formatters, but I think their impact on code history is negligible in the case of every legacy system I've worked on. The code history just isn't there. These aren't projects that used git until recently (if at all). Before that they used something else, but when they transitioned they didn't preserve the history. And that's if they used any version control system. I've tried to help teams whose idea of version control was emailing someone (they termed them "QA/CM") to make a read-only backup of the source directory every few months (usually at a critical review period in the project, so a lot of code was changed between these snapshots).

That said, sure, skip them if you're worried about the history getting messed up or use them more selectively.

SVN was a thing by the mid-2000's, and history from that is easy to preserve in git. Just how old are the sourcebases in question? (Not to shoot the messenger; just like, wow.)

edit:typo

The first large C++ project I worked on in mid-1990s was basically preserving a bunch of archived copies of the source tree. CVS was a thing but not on Windows, and SourceSafe was creating more problems than it been solving.

I kept regular tarballs of a project that used SourceSafe right near the start of my career, and found I was more likely to be able to find an intact copy of the right thing to diff against from my tarballs.

I think after a year or so I realised that even bothering to -try- to use SourceSafe was largely silly, got permission to stop, and installed a CVS server on a dev box for my own use.

(yes I know the VCS server shouldn't really be on the dev box I could potentially trash, I didn't have another machine handy and it was still a vast improvement)

I've heard a lot of stories about mid-90s codebases for sure

Some of these systems dated back to the 1970s. The worst offenders were from the 1980s and 1990s though.

It's all about the team or organization and their laziness or non-laziness.

I looked at a C++ codebase from 1997 at a previous job - I don't know much about the history but comments in one of the old files tracked dates and changes to 2001. Not sure what happened after that but in 2017 someone copy-pasted the project from TFS to git and obliterated the prior history.

I've had issues doing decent copies from SVN to GIT. They both have different ideas about user identity, and how fragmented it can be.

I maintain a C++ codebase that was originally written in 1996, and is mission critical for my organization. Originally maintained in Visual Sourcesafe, then in TFS source control, and now git. Some parts of it were rewritten (several times) in C#, but the core is still C++.

I was very worried when we transitioned to git that history will not be preserved and tried to preserve it, but it proved too much hassle so I dropped it.

In fact that proved not to be a problem. Well, not a problem for me, since I remember all the history of the code and all the half forgotten half baked features and why they are there. But if I'm gone then yes, it's going to be a problem. It's in a dire need for a rewrite, but this has been postponed again and again.

>I think their impact on code history is negligible in the case of every legacy system I've worked on. The code history just isn't there.

Not sure if I agree here or not - whilst yes, the history isn't there, if it's a small enough team you'll have a good guess at who wrote it.

Definitely found I've learnt the style of colleages so know who to ask just from the code outline.

Legacy systems that you inherit don't have people coming with them very often. That's part of the context of this. You often don't have people to trace it back to or at least not the people who actually wrote it (maybe someone who worked with them before they got laid off a decade ago), and reformatting the code is not going to make it any harder to get answers from people who aren't there.

I've been in situations where even without access to the people knowing which of them wrote something gives me a better idea of how to backwards infer what (and of course sadly occasionally 'if') they were thinking while writing the code.

Then again, I think most of the tells for that for me are around the sort of structure that would survive reformatting anyway.

(and, y'know, legacy stuff, everything's a bloody trade-off)

On per file level it's just 1 commit. It's not really a big deal

clang-format can be applied to new changes only, for this very reason.

Adding it will remove white space nitpicking from code review, even if it isn't perfect.

I believe you can configure `git blame` to skip a specific commit. But in my experience it doesn't matter anyway for two reasons:

1. You're going to reformat it eventually anyway. You're just delaying things. The best time to plant a tree, etc.

2. If it's an old codebase and you're trying to understand some bit of code you're almost always going to have to walk through about 5 commits to get to the original one anyway. One extra formatting commit doesn't really make any difference.

You can instruct git to ignore specific commits for blame and diff commands.

See "git blame ignore revs file".

Intended use is exactly to ignore bulk changes like auto formatting.

+1

  man git-blame
  git help blame

https://git-scm.com/docs/git-blame

This is another reason why you should track important information in comments alongside the code instead of trusting VCS to preserve it in logs/commit messages, and to reject weird code missing comments from being merged.

Not saying that fixes decades of cruft because you shouldn't change files without good reason and non-white space formatting is not a good reason, but I'm mentioning it because I've seen people naively belief bullshit like "code is self explanatory" and "the reason is in the commit message"

Just comment your code folks, this becomes less of a problem

How does reformatting trash the history? It's one extra commit..

I guess if it splits or combines lines that could cause some noise if you really want the history of a single line... But that happens all the time, and I don't see how it would really prevent understanding the history. You can always do a blame on a range of lines.

Maybe I'm missing something though, genuinely curious for a concrete example where reformatting makes it hard to understand history!

If you ask the IDE to show blame info next to each line, then a lot of lines will be from the big reformatting. If course you can dig in and retrieve the history still, but it's an extras step then. Btw, it seems that at least Git has a way to make `git blame` avoid considering certain commits (.Git attributes). Maybe that works in IDEs too!

I would absolutely not recommend auto-formatting a legacy codebase. In my experience large C++ projects tend to have not only code generation scripts (python/perl/whatever) but also scripts that parse the code (usually to gather data for code generation). Auto formatting might break that. I have even seen some really cursed projects where the _users_ parsed public header files with rather fragile scripts.

I was listing the items in the original article's #3 and saying I'd move them up to #2 before I'd go about excising portions of the project, the original #2. I still stand by that. But you can read my other comment where I don't really defend auto-formatting to see that I don't care either way. I made it about four hours ago so maybe you missed it if you didn't refresh the page in the last few hours.

CI is different from the others, here! At minimum, building a "happy path(s)" test harness that can run with replicable results, and will run on every one of your commits, is a first step, and also helps to understand the codebase.

And you're jumping around - and you'll have to! - odds are you'll have a bunch of things changed locally, and might accidentally create a commit that doesn't separate out one concern from another. CI will be a godsend at that point.

Nit: The post scopes "tearing things out" to dead code as guided by compiler warnings and unsupported architectures.

If going the route, I'd recommend commenting out the lines rather than removing them outright to simplify the diffs at least until you're ready to squash and merge the branch.

Better to use `#if` or `#ifdef` to prevent compilation. C & C++ don't support nested comments, so you can end up with existing comments in the code ending the comment block.

I think `#if` and `#ifdef` are not good ideas because they prevent the compiler from seeing them in the first place. A better solution is just `if (false)` which is nestable, and the code is still checked by the compiler so it won't bit rot.

Fair point!

Reply on news.ycombinator.com