Remix.run Logo
duskdozer 8 hours ago

if you mean better messages, it's not really that. those junk messages should be rewritten and if the commits don't stand alone, merged together with rebase. it's the "logical chunks" the parent mentioned.

it's hard to say fully, but unless a changeset is quite small or otherwise is basically 0% or 100%, there are usually smaller steps.

like kind of contrived but say you have one function that uses a helper. if there's a bug in the function, and it turns out to fix that it makes a lot more sense to change the return type of the helper, you would make commit 1 to change the return type, then commit 2 fix the bug. would these be separate PRs? probably not to me but I guess it depends on your project workflow. keeping them in separate commits even if they're small lets you bisect more easily later on in case there was some unforseen or untested problem that was introduced, leading you to smaller chunks of code to check for the cause.

orsorna 7 hours ago | parent [-]

If the code base is idempotent, I don't think showing commit history is helpful. It also makes rebases more complex than needed down the line. Thus I'd rather squash on merge.

I've never considered how an engineer approaches a problem. As long as I can understand the fundamental change and it passes preflights/CI I don't care if it was scryed from a crystal ball.

This does mean it is on the onus of the engineer to explain their change in natural language. In their own words of course.

seba_dos1 6 hours ago | parent [-]

Commits don't show "how an engineer approaches a problem". Commits are the unit of change that are supposed to go into the final repository, purposefully prepared by the engineer and presented for review. The only thing you do by squashing on merge is to artificially limit the review unit to a single commit to optimize the workflow towards people who don't know how to use git. Personally I don't think it's a good thing to optimize for.

orsorna 3 hours ago | parent [-]

Preserving commit history pre-merge only seems useful if I had to revert or rebase onto an interstitial commit. This is at odds with treating PRs as atomic changes to the code base.

I might have not stated my position correctly. When I mean "squash on merge", I mean the commit history is fully present in the PR for full scrutiny. Sometimes commits may introduce multiple changes and I can view commit ranges for each set of changes. But it takes the summation of the commits to illustrate the change the engineer is proposing. The summation is an atomic change, thus scrutinizing terms post-merge is meaningless. Squashing preserves the summation but rids of the terms.

Versioned releases on main are tagged by these summations, not their component parts.