Contributing to Crystal and questions about squash&merge strategy

Hello folks,

Noticed that contributions to Crystal main repository now follow a squash and merge strategy.

However, with this approach, details left in original commits that explain the reasoning on certain changes are lost.

One example one recent contribution in 11876:

While I understand the objective of clean history, not really sure if removal of what and why of the commits and forcing users to go to GitHub to read about them (and dig sometimes really long PR reviews).

Granted, the one-liner contribution of the example might seems trivial, but without the details of the change, people might not be able to understand why or its usage.

Found that this approach is not followed by other repositories within crystal-lang organization (Eg. shards), so wanted to check this before sending more PRs.

Thank you.

1 Like

Yes, losing the detailled history of each individual change is a downside of this approach.

It could be mitigated to some extend by copying the commit messages into the squashed commit. That just requires a bit more work (and paying attention) on merge, but it’s not hard to do.
This won’t break the change down in constituent commits, though. But for this, we can look at the pull request history. And that’s where you can find lots of information about how the change came to be the way it is. That and the related issue, of course. The fact is, this information lives on GitHub and can’t be easily ported into git. Nor would I expect that to be very helpful because many of these discussions are not very important to look up in the long term.

Permanently relevant information about implementation choices should rather be written as code comments, not in the commit message.
Then you have that in code, the commit message to describe the result of the change and the PR/issue discussions for full background information.

I think this model works reasonable well. At least it seems to fit better than any alternative. I was at first sceptical, when we discussed to squash merge only. But I have come to realize its value.

This model should also apply to other repositories in crystal-lang. I’m pretty sure we’re using it in shards.


Thank you @straight-shoota for your response. It was perhaps my old school way of doing things, was worried that the meaning of why certain change was introduced was lost, I’m fine with either way, specially if the taken approach can lighten the review effort and expedite the merges :grin:

As for other changes in crystal-lang org repositories I acknowledge my comment was mistaken as all the repositories I’ve interacted recently have been following it, apologies for the confusion, not sure what commit on which repository I was looking at when made that comment.

Once again, thanks for the time taken in providing a response.


Oh, I think it’s entirely possible there could be a merge commit in some crystal-lang repo. We’re currently not restricting the UI to squash merge. So on less frequently used repositories it could be easy to pick the wrong merge strategy because you’re used to just hitting the button (preference choice is kept per repo).