Cataclysm-dda: Consider squashing pull requests on merge

[fernzi]

C:DDA is a big, big project; this shows in the commit log, often showing merges, but with the actual commits nowhere to be seen, lost somewhere two weeks back, interspersed with commits from many other merges.

While this is an issue that cannot be fully solved without rewriting the project's history, it can be alleviated by squashing future pull requests on merge. This means that changes from a merge would be concentrated in a single commit, without polluting the project's history and, at least from that point on, creating a log that is much easier to read and search.

Also, it would slow down the repo's growth, I mean, I almost always make shallow clones, since the repo takes so long to download and occupies much more space than the game proper.

[AlecWhite]

Don't know if you want something like this https://github.com/CleverRaven/Cataclysm-DDA/issues/21325

[kevingranade]

I'm not clear what the use case here is, why are you reading the commit log
and how is squashing going to help (especially considering that if mergers
are squashing, they're unlikely to write good commit messages)?

On the other side of the coin, squashing can significantly hinder our
ability to bisect commit history to find the sources of bugs, and it also
requires more work on the mergers part, who are already the biggest
bottleneck for the project.

[fernzi]

I package C:DDA on the OpenSUSE Build System, which requires me to manually trigger the service that fetches the source from GitHub, so I read the log in order to be sure I'm not launching a rebuild during a major change (since those often get split across multiple pull reqs).

My main problem is that merge commits don't really contain any information other than the pull req title, which is not always useful, so I have to look at the rest of the log to know what was actually changed.

Squashing would eliminate this without any additional effort, since GitHub already supports automatically squashing merges; there is no need for the mergers to manually write commit messages, GitHub just compiles all the messages from the individual commits into one neat little summary.

Also, in matters of bisecting, I'd argue that having this level of granularity slows down the bisect → compile → test, in the sense it requires to sift through many tiny trivial commits instead of just reviewing proper changes.

[kevingranade]

It sounds like @alecwhite was right, and you really want the output of #21325. Possibly with an additional tag like "in-progress feature". Summary, its a system for automatically generating meaningful changelog.

I've been considering adding squashing to my merge workflow, especially in small-but-contentious PRs where it's all churn, but I'm not going to squash meaningful commits.

We don't use the github web interface for non-trivial merges since we don't have test coverage sufficient to have confidence that the PR does what it claims and that it doesn't introduce regressions, we need to manually build and test before pushing. As such squashing is non-zero effort.

The whole point of bisection is making finding regressions among many commits easy, reducing the number of commits doesn't reduce overhead significantly, and it reduces the quality of the output a great deal.

[fernzi]

Reading #21325 again, yes that would be delightful, and in my mind, squashing makes a great way for the git log become just that, a meaningful changelog.

From what you say, wouldn't it be acceptable to encourage squashing everything that would get merged through GitHub anyways? IMO most PRs are relatively minor changes, more so because big PRs are already discouraged, and often contain a lot of little commits that are only useful to keep in a working branch, like fixing typos and whitespace.

And yeah, what I wrote about bisect was kinda nonsensical. My idea was more about how squashing makes commits appear in the history when they became relevant in the code base, rather than when the developer pushed them in their working branch. Don't write comments when you're half asleep kids!

[kevingranade]

Squashing doesn't guarantee a readable changelog, the commits frequently
don't have that info. That's the key piece of that issue, it makes the
changelog message visible and reviewable as part if the PR.

Only about 1/20 or less of PRs are mergeable in the web interface, the
majority require local verification. I'm fine with encouraging people to
squash as much as possible, but I can't commit to always doing it.

[fernzi]

I see. My preference for squashing is more of an argument from beauty, while the underlying issue seems to be more about a lack of standards (and the enforcement of those standards) for commit messages than anything else.

I think that's something that'd be better discussed in #21325, so thanks for entertaining my idea. Feel free to close, if you think is pertinent.