Please disable the "Allow rebase merging" merge button #2726
Comments
bartlettroscoe
added
Framework tasks
bartlettroscoe
added this to the Improve productivity, stability, and quality of Trilinos milestone
|
If you're concerned about having to use I can think of scenarios in which you'd want to squash down to a small number of logically separate commits, but you'd want to keep them separate in the log. For instance, say you're refactoring a class, and in the process you decide to make some performance tweaks. Before a PR is merged you use All that being said, the frequency with which such scenarios arise for me is minimal. ~95% of the time I'm rebase-merging a single commit. And how you get people to only push logically separate, compilable, testable commits to a main development branch without mechanically preventing them from doing so is very much an open question. |
"Squash and merge" is identical to "Rebase and merge" when there is just one commit. I have never used |
bartlettroscoe
removed
the
client: ATDM
|
@trilinos/framework, There are people that are rebasing and pushing multiple commits with the option "Rebase and merge" button as seen recently in the example #3055 (comment). Note that GitHub appears not to let you revert these types of rebase and merges with the "revert" button. Also, git bisection that happens to hit one of these intermediate (untested) commits may have a false failure because it may not be a "complete" commit. Can we please uncheck the box "Allow rebase merging"? |
|
@trilinos/framework, Related to this Issue, I think people need some guidance about how to effectively do git bisection with Trilinos by bisecting only first-parent history on the 'develop' branch. It seems that even experienced developers who know git well have trouble effectively bisecting Trilinos. One example is #3438 (comment). To make bisection with Trilinos robust, we should disable rebasing commits for PRs (which this issue is about) and provide guidelines for how to effectively bisect Trilinos (e.g. along the lines described here). |
|
FYI: Here is an example of multiple commits for a topic branch rebased onto 'develop' instead of merged: It looks like these commits are from PR #3325 which got merged 4 days ago. If you look at the commits for this PR: you can see that the SHA1s for the commits are different. This issue is, is that if you try to bisection on @jmgate, did you select the "rebase" option or the "merge" option? If you only select the "squash" or the "merge" options, then this will not happen. |
|
@jwillenbring and @william76, Here are some examples of where people clicked the GitHub PR "Rebase" button for their PRs: For that last set of commits directly rebased on top of 'develop', good luck getting a successful build if It looks like you disabled the "Rebase and merge" button for Trilinos? So should we close this Issue? |
|
@jwillenbring just presented this at the TUG developer meeting and explained this to people. This is important to the ROL subteam workflow. Therefore, I think we can close this now. Thanks! |
@trilinos/framework
Can someone with the GitHub permissions please uncheck the "Allow rebase merging" option under the "Merge button" section at the page:
?
That will stop people from rebasing and then fast-forward merging a bunch of commits on the 'develop' branch. For example, currently, someone with 30 commits on their topic branch could rebase all of these commits and fast-forward merge them to 'develop'. If those commits badly broke an ATDM build for example, then we would have to back out 30 commits, one
git revert <sha1>at a time.By disabling this option, the options of "Merge" and "Squash" are still available, which is all we want/need.
If the topic branch has more than one commit, and they are well-formed "Separate Changes" (see workflows(7)), then you really should select the "Merge" option. But if you have multiple commits, and they are kind of a mess, then you might as well select the "Squash" option. (But people really should be cleaning up their topic branches before their final topic branch push as described here).
If you only have one commit on your topic branch, then it is silly to create a merge commit for this. In that case, you should select "Squash". When you select "Squash" with a single commit, GitHub populates the squash commit summary and log text the same as for the single commit. Therefore, it is equivalent to rebasing a single commit in this case. And a single squashed commit is as easy to revert as a single merge commit. In fact, a single squash commit is easier to revert because you don't need to provide the parent index with
git revert -m <sha>. You can just callgit revert <sha1>.Definition of Done
Possible Solution
The text was updated successfully, but these errors were encountered: