Project

General

Profile

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

Added by k0kubun (Takashi Kokubun) about 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
[ruby-core:94247]

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.

Related issues

Related to Ruby master - Misc #16093: Prohibit a "foxtrot merge" instead of a merge commitRejectedActions
Related to Ruby master - Misc #14632: [ANN] git.ruby-lang.orgClosedhsbt (Hiroshi SHIBATA)Actions
#1

Updated by k0kubun (Takashi Kokubun) about 1 year ago

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

Updated by k0kubun (Takashi Kokubun) about 1 year ago

  • Assignee set to k0kubun (Takashi Kokubun)

Updated by nobu (Nobuyoshi Nakada) about 1 year 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 1 year 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?

#5

Updated by k0kubun (Takashi Kokubun) about 1 year ago

  • Description updated (diff)
  • 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

Updated by k0kubun (Takashi Kokubun) about 1 year 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 1 year 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 1 year 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 1 year 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 1 year 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 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.
  • 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.
#11

Updated by k0kubun (Takashi Kokubun) about 1 year ago

  • Description updated (diff)
  • 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

Updated by ioquatix (Samuel Williams) about 1 year ago

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

#13

Updated by k0kubun (Takashi Kokubun) 11 months ago

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

Also available in: Atom PDF