Misc #16094: Allow only "Rebase and merge" or "Squash and merge" on GitHub master branch, and sync it on git.ruby-lang.org update hook

• Status: Closed
• Assignee: k0kubun (Takashi Kokubun)

[k0kubun (Takashi Kokubun)]

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.

[k0kubun (Takashi Kokubun)]

• Related to Misc #16093: Prohibit a "foxtrot merge" instead of a merge commit added

[k0kubun (Takashi Kokubun)]

• Assignee set to k0kubun (Takashi Kokubun)

[nobu (Nobuyoshi Nakada)]

"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".

[k0kubun (Takashi Kokubun)]

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?

[k0kubun (Takashi Kokubun)]

• 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)

[k0kubun (Takashi Kokubun)]

• 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.

[k0kubun (Takashi Kokubun)]

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.

[k0kubun (Takashi Kokubun)]

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.

[k0kubun (Takashi Kokubun)]

• 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].

[k0kubun (Takashi Kokubun)]

• 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 as git 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.

[k0kubun (Takashi Kokubun)]

• 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)

[ioquatix (Samuel Williams)]

It's a good idea since it can save a lot of time for maintainers.

[k0kubun (Takashi Kokubun)]

• Related to Misc #14632: [ANN] git.ruby-lang.org added