Hacker News Reader

Comment by st_goliath | Hacker News Reader

I guess this should be the revert he mentioned, for those interested:

https://github.com/curl/curl/commit/c2df780a97d9913eb20a55d4...

I always find it a small miracle that you can yank an individual git commit out of a program, and end up with a working program. If I commit anything, follow-up commits quickly make the original quickly a hard requirement.

Maybe it is a sign of bigger and more stable source code, that individual commits influence each other less.

Working on new, small bits of software often implies that a single commit will be more relevant to the rest of the codebase. The more code there is, the less any one particular line, on average, would matter.

Making smaller and atomic commits makes them more easily reversible.

Atomic meaning they can be reverted on their own; I'm sure plenty of people make the mistake of making small commits, but not atomic commits.

If your git history looks like a work log, you're not writing atomic commits.

And people should get comfortable with git commit --amend and interactive rebase.

I'm a nobody, but surprises me that the revert (and so the original feature) didn't include any tests. I always imagined that a project like curl (no deadlines? no pressure? no bosses?) would lead to people to work on it at their own pace (so more tests maybe? Maybe not, I don't know)

It was a performance optimisation, so I suppose it should be covered by existing tests as there should be no change in behaviour

Ah, good point!

This looks like an integration point, which are pointless to test.

It did in-fact have test coverage, from tfa:

> Apart form the little bug that caused it to crash in several test cases. (...)

> Exactly why this was not discovered in our tests and CI jobs before the release we have yet to figure out, but it is certainly more than just a little disturbing.

Reply on news.ycombinator.com