Hacker News new | ask | show | jobs
by superbatfish 1740 days ago
I agree: Many people have half-baked commit messages. And many erroneously assume that a PR is always a single atomic code change, rather than a sequence of pseudo-atomic changes.

As an industry, we should be nudging people away from those bad habits. Instead, the "squash merge" feature nudges them in the opposite direction. It disincentivizes careful commit commit messages and encourages general disregard for the commit log.

Collaboration requires good communication. For programmers, that means:

- writing clear code

- commenting your code

- documenting your code

- writing a clear summary in the PR description

- organizing your changes into cohesive, (mostly) self-contained, reviewable units -- expressed as independent commits in the version history

On that last point: The commit log is something worth curating, tending to it with the same care you would use for the other points.

Yes, PRs are often small and self-contained enough that it shouldn't be subdivided into multiple commits. But just as often, it's clearer to use multiple commits which can be reviewed in sequence, rather than in one big chunk.

In the latter case, it's incumbent on the PR author to present a logical, clean commit history. If you wrote four commits but two of them are inseparable, then squash those two before you submit the PR. Commit messages like "fix typo from last change" are completely unacceptable in a PR. That sort of thing is fine in your own repos, outside of any review process. But when sharing code with others? No way -- that's inconsiderate with others' time, and pollutes a valuable data stream (the commit log).

But hey, if there's a good chance that all your commits will be squashed anyway, then why bother curating your commit log at all?

Instead of letting bad commit messages slide and simply squashing them, we should gently insist that PR authors fix their commits on their end. Their next PR will be better for it.

In the past, I've been occasionally annoyed when my carefully crafted commit log was simply squashed by a PR. I had intended the commits to be reviewed one-by-one. And if a bug is discovered in one commit later on, it would be clear which lines ought to be reverted, and which should stay unchanged. All of that care gets thrown out by a squash. Grr...