[ruby-core:94237] [Ruby master Misc#16093] Prohibit a "foxtrot merge" instead of a merge commit

Issue #16093 has been updated by marcandre (Marc-Andre Lafortune).


Another +1 for no merge commits.

> Therefore, we have not been able to make a pull request "Merged" when a pull request's branch needs to be rebased before pushing it to the master branch.

I'm probably missing something, but Github allows rebasing from the UI.

> But force-pushing the rebased commits to their author's branch is a bad habit and we do not want to do so.

I'm probably missing something too, but rebasing and force-push a branch used for a PR is a good habit.

 

----------------------------------------
Misc #16093: Prohibit a "foxtrot merge" instead of a merge commit
https://bugs.ruby-lang.org/issues/16093#change-80543

* Author: k0kubun (Takashi Kokubun)
* Status: Open
* Priority: Normal
* Assignee: 
----------------------------------------
## Background
* When we migrated the canonical Ruby repository from Subversion to Git [Misc #14632], in that ticket nobody had objected to allowing a merge commit in the repository.
* At first, we decided to prohibit merge commits because:
  * The first merge commit https://github.com/ruby/ruby/pull/2084 went well. Then we tried the second merge commit for https://github.com/ruby/ruby/pull/2079 but it failed.
  * We struggled to find why only the first one succeeded. That was tricky because ruby-commit-hook's pre-receive hook was updated to a new revision *after* the first merge happened, and the new revision included a change that accidentally made a merge commit impossible.
  * Because the merge commit made it harder to debug the issue in ruby-commit-hook, we decided to deliberately [prohibit](https://github.com/ruby/ruby-commit-hook/commit/d7759cca6282741ecc9c46053166a1f5f5779c10#diff-7e61afe505452158c45dfa32ea7d7a14) to push a merge commit to the master branch for a while to reduce the number of problems to be solved during the early stage of the migration to git. These days the ruby-commit-hook has worked stably.
* Then we also noticed that prohibiting merge commits makes it easier to efficiently list up revisions to be built by the bisect bot [rubyfarmer](https://github.com/mame/rubyfarmer) without having a data store.
* If we do not make a merge commit, there's only one way to make a pull request "Merged" on GitHub ruby/ruby:
  * Push commits in the pull requests to the master branch when their parent revision is the same as the master branch's one.
* Therefore, we have not been able to make a pull request "Merged" when a pull request's branch needs to be rebased before pushing it to the master branch.
  * But force-pushing the rebased commits to their author's branch is a bad habit and we do not want to do so.

## Problem
* Some contributors get confused when their pull request is committed to the master branch but it's marked as "Closed" on GitHub [Misc #16089].
* Even though we clarified the situation in https://bugs.ruby-lang.org/projects/ruby/wiki/HowToContribute, a first-time contributor could be confused and the person might complain about it to the committer.

## Solution
1. Improve the rubyfarmer's implementation to make it work even if we had merge commits. https://github.com/mame/rubyfarmer/pull/1
2. To maintain a consistent linear history in the git log even after allowing merge commits, implement a guard to prevent a ["foxtrot merge"](https://blog.developer.atlassian.com/stop-foxtrots-now/) in the pre-receive hook. https://github.com/ruby/ruby-commit-hook/pull/19
  * Details: https://devblog.nestoria.com/post/98892582763/maintaining-a-consistent-linear-history-for-git
3. Fix bugs in [check-email.rb](https://github.com/ruby/ruby-commit-hook/blob/master/bin/check-email.rb) to correctly check merge commits.
4. Allow pushing merge commits to the master branch.

We'll implement 2, 3, and 4 shortly. Please let me know if you have any opinion about this.



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>