ImperialCollegeLondon / R2T2

Research References Tracking Tool
MIT License
14 stars 155 forks source link

Suggestion: Always use squash merge #56

Closed de-code closed 4 years ago

de-code commented 4 years ago

There are a number of advantages when using squash merges. I could find some good reference explaining it. But some simple ones would be:

Individual PR commits to say fix a previous PR commit or merges from develop into the PR branch do not add value and make the commit history more noisy. The commit history is easier to read if it just lists the merged PRs (which then include some comments for further clarification).

In the GitHub project settings you could disable the other two merge options, just leaving squash merge.

ChasNelson1990 commented 4 years ago

I don't use squash merges so apologies if this is wrong but doesn't this completely break the idea of atomic commits and thus prevents cherrypicking and patching? I guess these maybe aren't used that often... but it's nice to be able to when you need to.

dalonsoa commented 4 years ago

I would not enforce it. During the squash process you loose information, and that's something to be avoided if possible. There are specific cases where it makes sense - as @de-code points out - but it should not be the default.

de-code commented 4 years ago

I think the key is to have small PRs. You can then certainly cherry pick those. And I believe you could cherry pick individual commits if you need to while they are still available.

Just some references:

Currently the commit history is really not easy to read and there are even broken builds (to which I contributed). I am not sure what value they add.

rgaiacs commented 4 years ago

I'm don't like the idea of enforce squash merges as you loose information in the process (for example, how did someone fix something).

de-code commented 4 years ago

Closing as I seem to be the only person in favour of this.