-
-
Notifications
You must be signed in to change notification settings - Fork 554
Decide merge options for repo #1934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
In other larger repos like the former v3 repo it was rare that a PR was in a state where it shouldn't have been squashed. Given that the vast majority of PRs in this repo only edit one exercise, I imagine it'll remain similar here. However, in those rare cases there's still the option to have it rebase-merged by an admin as they can override the settings that are available. |
This is also my preference, given that we do sometimes have PRs with multiple commits that we'd like to merge as individual commits.
A key question is: how often does this occur (having a discussion on the merge strategy)?
This could also work., but would effectively make admins a bottleneck. Not to say that I think that won't work in practice, but just wanted to point it out. |
I think the main concern is the opposite. This discussion doesn't happen, someone merges without squashing, and the commit history gets cluttered (see the recent commits) |
Ah, that is a very good argument. |
@exercism/reviewers Thoughts? TL;DR disabling the merge and rebase merge options forces all PRs to be squashed, preventing any potential messiness in the commit history. The downside being that for those cases in which we do not want squashing, an admin (Jeremy or I) have to do so. |
I prefer the well curated git history that can be afforded with the rebase, but that does not always happen (perhaps more often than not we have contributors that do not realize what that is, which is fine) and the ability to revert a part of a patch-set that proves troublesome, rather than an entire patch that has to be broken down. But when in Rome, which I have never been, but one day hope to go, perhaps I would wear a toga. (Or is that Greece?) |
I'd prefer the option of a rebase as well -- but am happy to go with the majority opinion. 😄 |
I hate rebases when I have PRs from PRs. I don't do that in this repo, but I do in others. That said, I would be okay with squash-only, as even though I think sometimes we really show regular merge / rebase merge, the times that happened are soooo little compared to squash being the best choice, that I think in those cases we'll be okay cherry-picking a PR into multiple ones if we really care. |
I am very happy with only squash-merges being allowed. If there's only one option available, there is no room for accidentally choosing the wrong one or not giving it enough thought. I have had exactly this situation at work for the last 3 years. In rare cases when we are interested what separate commits were actually in a specific PR, GitHub still has all of this data available for all PRs. The only problem we experienced with always squash-merging is, in rare cases, when somebody uses |
Okay. Having heard from a lot of people (thanks all!), let's make this a squash-only repository. If we ever want to merge/rebase, ping me and I can override it. |
We've disabled the non-squash options. Thanks all! |
A discussion was starting in #1933, but it should be moved into its own discussion otherwise we won't be able to find it from the title.
Originally, I reacted harshly to the idea that squashing should be the only option. But after considering it, I realise I should not be so harsh. It is still my preference that a non-squash option remain available, in case there are instances where we do not want to squash. However, given this is a repo shared among many, if the consensus is that the advantage of only enabling squashes outweighs the drawbacks, I accept that consensus.
Advantages: simplifies decisions and avoids questions of "should this be squashed" - if it's the only choice, no more questions!
Drawbacks: Makes contributors' lives harder in instances where they wish to contribute commits that should not be squashed - they'll be forced to make one PR for each.
The text was updated successfully, but these errors were encountered: