Misc #16094
closedAllow only "Rebase and merge" or "Squash and merge" on GitHub master branch, and sync it on git.ruby-lang.org update hook
Description
Problem¶
- Our pull request merge strategy confuses contributors, as described in [Misc #16093].
- In [Misc #16093], some committers did not like accepting a merge commit.
Solution¶
- Allow using GitHub's "Rebase and merge" (suggested by @marcandre (Marc-Andre Lafortune)) and "Squash and merge" (for editing a commit message as needed) to committers
- Disable "Create a merge commit" in GitHub repository settings, to avoid concerns found in [Misc #16093].
- Also prohibit pushes to the master branch which are not coming from a pull request by requiring CI in GitHub repository settings.
- To allow looking up an associated GitHub pull request from git commands, add
git notes
to "Rebase and merge"d commits in git.ruby-lang.org post-receive hook. (suggested by @nobu (Nobuyoshi Nakada))
- Perform GitHub bidirectional sync on git.ruby-lang.org update hook, and accept a push if it does not conflict with "Rebase and merge" on GitHub.
Notes¶
- Even after implementing this ticket, the Ruby's canonical Git repository will continue to be git.ruby-lang.org as is.
- All committers must push non-pull-request commits to git.ruby-lang.org. Direct pushes to GitHub will continue to be disabled.
- This approach has one drawback;
git push
to git.ruby-lang.org might block longer than now.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Related to Misc #16093: Prohibit a "foxtrot merge" instead of a merge commit added
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Assignee set to k0kubun (Takashi Kokubun)
Updated by nobu (Nobuyoshi Nakada) about 5 years ago
"Rebase and merge" makes the target commits unable to track the original pull request from git log
.
So I propose "squash and merge", or pushing a marking commit at the sync after "rebase and merge".
Updated by k0kubun (Takashi Kokubun) about 5 years ago
I'm personally okay with "squash and merge" too, especially because "rebase and merge" cannot update a commit message when we want to add a thing like [Bug #12345]
. We can allow both "rebase and merge" and "squash and merge", and make either of them ruby/ruby's default.
@jeremyevans @marcandre (Marc-Andre Lafortune) Do you have any preference on it? Or do you think it's fine to make "squash and merge" default, as it keeps a true linear history too?
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Subject changed from Allow only "Rebase and merge" on GitHub and sync it on git.ruby-lang.org pre-receive hook to Allow only "Rebase and merge" or "Squash and merge" on GitHub, and sync it on git.ruby-lang.org pre-receive hook
- Description updated (diff)
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Description updated (diff)
As @nobu (Nobuyoshi Nakada) wanted to know which commit is associated to which pull request using git without accessing GitHub, he experimented git notes
a little bit, and we found that we could automatically attach notes to "Rebase and merge"d commits on our git.ruby-lang.org's post-commit hook.
So probably "Squash and merge" is not mandatory for his purpose, and it seems fine to make "Rebase and merge" default.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
we found that we could automatically attach notes to "Rebase and merge"d commits on our git.ruby-lang.org's post-commit hook.
I implemented this in https://github.com/ruby/ruby-commit-hook/blob/d05f66c9df2eabe45f7cb1d8bd3d51c356144e23/bin/notes-github-pr.rb.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
Experimentally, I'll move GitHub sync from post-receive hook to update hook, which blocks git push a little more. Still most of the behavior would be the same, but you might feel git push became slower.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Status changed from Open to Assigned
This is almost implemented. It's still under testing and only available for admin now, but you may see some usages for testing. I'll add one more guard and then enable "write" to merge pull requests for ruby-committers team.
I also noticed that I cannot choose either "Rebase and merge" or "Squash and merge" as the default merge behavior. When enabling both, "Squash and merge" is made always default. So it's shown by default. Of course, "Create a merge commit" mode is disabled as we discussed [Misc #16093].
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Status changed from Assigned to Closed
While this feature is still experimental, I wrote a script to automatically check the consistency between Git repositories every 10 minutes. So it's somewhat safe now, and we'd be able to always restore the repository from logs. I'll take the responsibility to do the recovery if something happens...
Therefore, I enabled "write" access to "ruby-committers" team to use the GitHub merge button with either "Squash and merge" and "Rebase and merge". As discussed in [Misc #16093], a "Create a merge commit" is disabled. It's available only for master branch, and other branches are protected by CI.
Here's the summary:
What's changed?¶
- You can push "Squash and merge" or "Rebase and merge" button on a GitHub pull request, but only for master branch.
-
git push
to git.ruby-lang.org became slower, almost as slow asgit push
to GitHub. I'm really sorry about this.
What's not changed?¶
- Committers should continue to use git.ruby-lang.org for usual
git pull
/git push
destination. - Nothing is changed in branches other than "master".
- "master" is the only branch which is synchronized bidirectionally.
- "trunk" and "master" are mirrored each other (until 2020/1/1, as said in [Misc #15843]), but pull requests can be merged only to "master". Of course, you can still push commits to "trunk" branch from git.ruby-lang.org as before.
- You cannot directly push to GitHub ruby/ruby master branch, except "admin" users.
- We need to allow this to admin users because matzbot (admin) needs to sync commits to GitHub.
- While
git push
to GitHub ruby/ruby master branch would work by bidirectional sync, to simplify the problem, admin should not push commits directly to GitHub. - "admin" users: ruby/ruby owners (@matsuda (Akira Matsuda), @hsbt (Hiroshi SHIBATA), @mame (Yusuke Endoh), @naruse (Yui NARUSE)), @k0kubun (Takashi Kokubun), and matzbot.
- You cannot use a merge button against non-master branches, except "admin" users.
- Pushing whatever commits to non-master branch would introduce inconsistency, just as before (this fact is NOT changed in this ticket). Be careful, "admin" users.
- A merge commit will never be created unless the above "admin" users intentionally push merge commits to a GitHub branch directly.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Subject changed from Allow only "Rebase and merge" or "Squash and merge" on GitHub, and sync it on git.ruby-lang.org pre-receive hook to Allow only "Rebase and merge" or "Squash and merge" on GitHub master branch, and sync it on git.ruby-lang.org update hook
- Description updated (diff)
Updated by ioquatix (Samuel Williams) about 5 years ago
It's a good idea since it can save a lot of time for maintainers.
Updated by k0kubun (Takashi Kokubun) about 5 years ago
- Related to Misc #14632: [ANN] git.ruby-lang.org added