Qmk_firmware: Discussion about pull request squashing

[fredizzimo]

I have noticed that you have started to apply pull requests by the new GitHub squash feature.

I'm not really a fan of that, and would like to provide a few reason why I think it's not a good idea. But I also acknowledge the fact that it cleans up the commit history, and it's easier for end-users to find out exactly what has been modified, so I'm not requesting that you change your ways of doing things, unless you agree with my comments.

I personally use the version control history a lot, when trying to find out the reason why something has been changed, or written in the way it is. In Git, that tool is git blame, but in Perforce, which is the version control system I use at work, the tool is even better, and called time lapse view. If the commits have good, and descriptive messages, they can give a lot of insight. In most cases this is better than in-code comments, since comments could get stale, while the commit message describes exactly what was going on at that exact moment.

I also use the commit messages, or diffs for finding out bugs. I have found countless bugs, this way, and in many cases it's way more effective than starting the debugger and figure out what the current code does.

Git bisect is another way of finding bugs, since you can quickly be taken to the exact change that broke something.

However due to the squashing, the commits will be large, and you will have a huge diff, with a rather non-descriptive message, or a commit with a lot of different messages. This makes doing all the above things much less effective.

Another issue is, that pull request or features could be written by many authors, like the case for the main ChibiOS pull request. Flabbergast had written most of the actual code, while I was the one that got all the credit due to the squashing.

The Github contributor statistics will also be skewed, since someone that does just a small change, and someone that makes a big feature, gets the same amount of commits.

Squashing in GitHub doesn't destroy the information though, which is very good, so it's not a huge drawback, and I'm able to live with that. You can for example go to the actual pull request, to browse the commits. Or you could check out the pull request locally.

For a lot more arguments, both for and against the squashing, look here

[algernon]

I'm with @fredizzimo here: as someone who uses version control a lot, squashing my PRs makes my job a little bit harder, because it becomes considerably harder to follow what has been merged, and what hasn't yet, unless I keep my PR branches around for a longer period of time.

Reason for this is that I have a (private) branch where I do all my changes, and cherry-pick cleaned up patches to a PR branch. If the history there is squashed, that confuses my workflow. It also pretty much kills git bisect, which is an incredible aid in figuring out when and what broke something.

Thankfully, as far as I remember, GitHub allows to merge without squashing, even if squashing was made default. So perhaps it would be an option, for those PRs that are pretty clean anyway, to merge them without squashing, and squash the rest? Or, even easier: if the person opening the PR requests the PR te be merged without squashing, perhaps that could be a reminder to consider doing so?

[jackhumbert]

Yeah, I'd definitely like to stop squashing most PRs - I originally started because a lot of the commits were messy/bad (especially on my part). Sorry if I caused any offense by squashing yours ;) I'd be happy to refrain from squashing you guys' PRs in the future!

[fredizzimo]

Thanks,

And if you think that a pull request is too messy, you can always ask the author to clean it up. At least I'm happy to rebase and cherry pick until it's good enough. But of course everyone is probably not confident enough with GIT to do that.

[ezuk]

Looks like we're all in agreement here! Closing this one.