Hacker News Reader

Ideally it should be this way, but it's impractical in reality.

It requires that you either stop your development workflow to commit as you go along, or that you untangle all the pieces after they're already entangled.

If you commit as you go, it's an expensive mental switch to fire up git and also run all the tests (since surely part of this workflow is to apply the principle that no commit should ever break the build). You also take an extra productivity hit every time you change your mind about something a little later (e.g. you added the function getFoo() but realize it should have been called findFoo()).

If you work for a while and then try to bundle up small, atomic changes, that can also be very difficult. Tools like git group together contiguous chunks of changes when committing, and prying them apart later can be difficult. I often do this with a combination of "add -p" and then "stash save -k" to temporarily get rid of things unrelated to what I'm committing, but it's a chore. During a selective "add -p" session you have to mentally keep track of what belongs together, thus what dependencies are between every chunk you're adding.

Committing as you go is easier, but it's slow, and doesn't work well when you're working across many files with a big change that introduces new semantics in a lot of places. Both techniques require that you keep track mentally of which parts are related, of course.

Untangling is what I mostly do. I consider the untangling my own internal code review. I need to read my own diff and figure out what goes where and what each part does and why it's necessary. My commit messages are then my own code review comments.

I figure if I don't carefully read my own diff, why would anyone else? And once it's untangled, I am hoping others will find it easier to read too.

Git doesn't provide as many tools as I would like to make this process easier. It's partly why I don't use git. Mercurial's absorb command helps a lot: it absorbs changes from your working directory into the appropriate draft commit that corresponds to the same context:

https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-ch...

Wait, it appears someone finally ported it to git:

https://github.com/tummychow/git-absorb

That's a cool script. I will definitely try that. Augmenting commits by doing partial commits then fixing with "git rebase -i" and squashing with "fixup" takes so much time and mental effort just to not make a mistake.

It still doesn't solve how to disentangle changes that have become interdependent. For that you have to concentrate on committing atomically and planning ahead a lot.

To toss in a counter-point: I do commits as I go and occasionally go back and make changes so it's a coherent sequence. For the most part, once you are fluent with Git[1], I've found it to be a productivity improvement, and code reviews have been both faster and more useful.

If you're doing two semantically different things, put it in two different commits. If it's one, put it in one (merge commits work too). That's just good change-hygiene, for the same reasons you try to isolate behavior in code, rather than mashing it all together into a single func just because you happened to be doing it all around the same time.

tl;dr we don't name our funcs "june_27_through_29", don't name your commits like that.

[1]: a huge investment, so I totally get why this isn't an early-coder practice, and it's rather painful. but IMO worthwhile, usually I see people spending far more time fighting it than it would take to learn it.

I don't think this is impractical.

I've been using this approach successfully for 8 years now on tens of open source projects and various company code bases of all sizes.

It does take a small amount of overhead (I measure this, and for me it's around 5%). But that pays off immediately as soon as you or someone else reads it a few weeks later when investigating an issue.

I use gerrit for everything and this workflow is exactly what it gets you (well, to be clear, my workflow is commit-per-issue resolved, not commit-per-function added, though you could use it that way also). I highly recommend it or a similar tool.

I wrote a post three years ago about my switch:

https://techspot.zzzeek.org/2016/04/21/gerrit-is-awesome/

While I am following this conversation closely, I wanted to politely make a suggestion about something you said:

"You misunderstand me. Almost everyone does."

That sounds really frustrating. It's not clear from your post whether you mean this as "everyone who reads this comment does not understand me" or "people frequently misunderstand me"... but typically, someone who feels this way experiences this in the latter, general sense.

Good news: it is possible to dramatically improve the ratio of people who understand you. It will, however, require that you change how you communicate some kinds of information.

What I'm saying is that if people don't understand you, the first place you need to look is your own tools for communicating with people. Of course, what you are saying makes sense to you. But there's an intellectual fallacy at work if you assume that "almost everyone" is the problem and you are the solution. :)

Have you ever heard the saying, "The only thing all of your crazy ex-partners have in common is you?"

There are people who are tasked with explaining far more complicated concepts than source control who people claim to love learning from. That means there is hope for us all.

Yes, I know I'm not communicating effectively. I shouldn't have implied that everyone is wrong but me. I know that there are people who understand me, but in a world of github and pull requests, it's really hard for me to explain what the world looks like otherwise.

There is a name for what you are describing, it's called "atomic commits". Every commit can stand alone and at every commit the software should be in a working condition.

Gary has a talk about this: https://www.destroyallsoftware.com/screencasts/catalog/sourc...

I mostly agree with you, but I think this might be going a little too far:

> You created a new function? That's one commit. Take a moment to explain why this function is going to be useful in future commits.

> You called that new function from several new spots? That's another commit. Explain why each of these calling sites requires this function.

In my opinion each commit should make sense on its own. It doesn't really make sense to create an unused function, so these two changes should really be one commit.

Maybe. I think both ways can make sense, and if you insist on having the function and the calling sites in the same commit, that can make sense.

I think my proposal can also make sense because (1) it splits up the commits into two units that still keeps the codebase in a stable state [this is my rough metric for what "atomic" means] and (2) defining a function requires some independent contemplation about why that function is defined that way. Inserting that function into calling sites can be a logically distinct operation if the calling sites are varied and distinct enough to each have a different reason to now require this function.

Thus the two commits can be semantically distinct.

I see this the same way. Like the notion that purely cosmedic changes should be committed distinctly in order to not clutter up commits that change behavior (so they are simpler to read), it makes sense to me that one would commit entirely new units of code separately before re-wiring the logic of the existing code accordingly. That way the first commit basically states "I've built this new thing", and I don't have to consider 300 random existing codepoints in understanding what it does.

that doesn't scale. I personally prefer a single squashed commit linking to a full discussion in a PR/MR: do a git blame, even if you get a giant hairball commit, you should be able to trace it to a review process (PR/MR) where it was discussed and thoroughly reviewed.

It scales quite well. Linux itself is developed in this way. Or perhaps you think Linux isn't at a large enough scale? (No sarcasm, I know that there projects out there much bigger than Linux.)

I think the extra work for a single developer to perform atomic commits is justifiable.

How does this work with multiple developers working on the same repo? I'm assuming everyone should work on their own feature branch and send PRs once their branch is done? Should the commits also be tagged by the feature branch they're on? Should the CI approval workflow be run against any combination of commits on the feature branch or against the final HEAD?

With git, I think it helps to think of development in terms of patch series.

An individual commit is a single patch, intended to do one thing (and hopefully do it well), and a feature branch is a patch series.

A pull request is then a request to review the series. If you need to change things, git allows you to rewrite your commits to send in a revised set of patches.

Before merging, your CI would create a temporary branch off the current master, merge your feature branch to that, and run tests against the result. I don't think testing individual commits (fully, at least) in a series makes much sense if you're going to merge all of them to master anyway.

I'm not sure about "scale" but Linux, as an open-source project where external actors want their code included, can push arbitrary amounts of work onto those actors with no additional cost to itself.

They can say "to get your code upstream you have to do twice as much work" or 3x or 4x or whatever. It's not their cost to bear.

An internal team pays that cost. They have to consider whether the trade off of having a pristine commit history is worth the additional overhead of doing it.

I personally care more about PR size and am happy to squash all commits in a PR. If that is too big I'd rather see multiple smaller PRs.

Not sure why this was down voted. Squash’n’Merge on Pull Request keeps master history clean while tolerating sheninagins on feature branches.

If important things are lost then perhaps the PR was too large.

To do this effectively, we have to change the way we edit code, so it's an editor/workflow feature rather than an scm one. Trying to solve it downstream in scm is not productive.