[policy] Define standard for merging in branches from core sandboxes

Closed #1462712: Discuss best practices for managing core merges as duplicate. Here's what I said over there:

Rebasing the main sandbox/merge branch is probably not a good idea because it alters the actual history of the branch development.

However, if we are going to do more of these merges in the future, here's the workflow I'd suggest:

• Have a main branch for the sandbox, the one we intend to merge into core.
• Branch off this main branch when making changes, possibly maintaining a parallel "development" branch.
• When a change is "ready," rebase or squash any number of commits from the change back into the main branch, in a single "nice" commit with a helpful diff, more like we have in core currently.

This would also apply to individual developers making a self-contained change. The change might include multiple commits in that developer's branch, but the developer would squash it into a single commit when merging back into the main sandbox branch.

I'm still not sure I agree about rebasing the main branch against 8.x before the merge. What about commits that are in response to changes in core? If you rebase, suddenly those commits are out of context.

One huge squash merge also offers zero advantages over just having a patch. You get no history, no context, no nothin'. The history looks no different than having a patch file, so we gain nothing. That doesn't mean we shouldn't rebase out some commits, but we should not destroy the entire history.

We should also consider that making a "tidy" merge branch and then merging it is extra steps for the branch/patch submitter and committer need to synchronize on. That could easily add an extra week to the time-to-commit of an issue as that gets sorted out. That harms development velocity.

I will also note that the "log doesn't make sense" issue is only true if you're using a linear log view. If you're using a branch-aware log view (gitk, gitX for Mac, or just git log --graph), you get a more accurate representation of the history. Really, a linear log for a non-trivial git project makes no sense in the first place unless you're just looking at a small part of recent history.

To clarify, I did not mean "one huge squash merge"--I meant many small squash merges into the main sandbox branch, and then a single full merge from that branch into core HEAD.

Edit: E.g.:

(On branch 8.x-thingy-xjm)
Fix spelling.
Missed one.
Add some more.
Whitespace.
Add foobar widgets.

Get squashed into:

(On branch 8.x-thingy)
Add foobar widgets.

Which gets merged into:

(On branch 8.x)
Merge remote branch '8.x-thingy' into 8.x
...other commits...
...other commits...
Add foobar widgets.
...other commits...
...other commits...

I should note this is similar to the workflow we use at work, so I'm partial. :)

Edit: Also, it sounds like Crell is thinking this is something that would be done at the end of development. I'd disagree with any proposal along those lines. I am saying a small-scale part of the daily workflow that involves bundling fixing one's whitespace and typos in something with the work on that thing before one foists that commit off on other devs. :)

Rebasing the main sandbox/merge branch is probably not a good idea because it alters the actual history of the branch development.

No, the rebase changes the history of the (temporary) branch to be merged upstream only. The original development branch(es) can happily remain as is.

rebasing destroy's history. end of story. dates are changed, order is rearranged, history is changed. If you're on a private branch and preparing a patch or even a series of patches this destruction is sometimes acceptable. With large changes it is not acceotabke, this is how weird regressions happen.

While a straight chronological view can get a /little/ weird with merges, they are actually more clear because they show when actual development was happening and tools like gitk, giggle and the others crell mentioned can help you inspect when, where and why regressions happened which would be completely impossible with a rebased branch.

As far as having 2 branches for changes, this doesn't actually help it, if anything it makes it worse because there's no way to actually tell that to resolve some commits' history you actually need to look on a different branch to get the full history. As I said before, history is lost.

edit: fix quotes

I pretty much completely agree with everything Crell said. I'm not necessarily against rebasing out some pointless commits, but on the other hand I do think it is a little bit of a waste of time on the part of the maintainers managing those sandboxes. Git inherently lends itself to small atomic commits, and I'm going to be less likely to do them if I know I'm going to have to go through and clean them all out again later. This doesn't mean that perhaps we shouldn't all become a little more cognizant of our commit messages and cleanup commits (I haven't really been giving that much thought to be honest.) I completely agree that one squash-merge is a total waste. Note that at the very beginning of the initiative process, I opened this topic up for discussion here:

http://groups.drupal.org/node/148184

A lot of that discussion is still directly relevant.

I plan on having CMI merged into core. The 20-odd contributors to the branch deserve full credit and I want to see them get it.

What we need to learn here is how to build patchsets.

The history of the trial and error of the developer working on a patch has absolutely zero value. On the other hand, splitting a big change into separate smaller changes that make sense in isolation and can be reviewed separately has a lot of value.

In that particular case, I don't see how a proper patchset would have been done (it's basically moving a lot of code around), but at the minimum the merge history of the development branch should have been removed:

| *   1bc5b98 Merge remote-tracking branch 'dries/8.x' into dbtngtng
| * |   e9453d8 Merge branch '1400748-namespaces' into dbtngtng
| | * \   c616a7b Merge branch '8.x' into 1400748-namespaces
| * | | |   a731ac1 Merge branch '1400748-namespaces' into dbtngtng
| | * | |   89a813c Merge branch '8.x' into 1400748-namespaces
| | * | | |   0e7882b Merge remote-tracking branch 'dries/8.x' into 1400748-namespaces
| * | \ \ \ \   b5e33f6 Merge branch '8.x' into dbtngtng
| * | | | | |   0ee2386 Merge branch '1400748-namespaces' into dbtngtng
| | * | | | |   910535c Merge branch '8.x' into 1400748-namespaces
| * | | | | | |   86c93f3 Merge branch '1400748-namespaces' into dbtngtng
| * | | | | | |   3f0cb72 Merge branch '1400748-namespaces' into dbtngtng
| * | | | | | |   adbd7c9 Merge branch '8.x' into dbtngtng
| * | | | | | |   0879ffd Merge branch '8.x' into dbtngtng
| * | | | | | | |   0017077 Merge remote-tracking branch 'dries/8.x' into dbtngtng
* | | | | | | | | |   3c78ca4 Merge branch '8.x' of git.drupal.org:project/drupal into 8.x

I have a hard time disagreeing with anything Damien said. We definitely as a community don't understand managing development branches or auto merge commits. Education and probably some helpful process guidelines for getting a clean merge to core are inline. Maybe we're all arguing the same point and disagreeing on the semantics. Thanks Damien.

Updated issue summary.

I use the --no-ff flag to accomplish something similar to this type of grouping. It gives us the ability to quickly evaluate a group of merges for a feature and keep them grouped together so they can be targeted efficiently. Each developer uses a regular merge as work is happening with the release manager using a merge --no-ff. This has proven to be invaluable where I work.

We've merged from sandboxes a few times now, and no one's bumped this issue in almost a year, I'm going to go out on a limb and say it's not "major" anymore.

It would be good to get this wrapped up one way or another, though.

Also, feels like documentation to me.

Seems realistic though I'd like to see more interest in things like this. I feel like passing up and ignoring things like this leads to things like this https://twitter.com/Dries/status/288682798636756993 not happening.

disclaimer: I'm not in a position to really complain as I'm in as much of a time crunch for community efforts as anyone else I just wanted to voice my opinion on the importance.

Backlink to #1583840: Define conventions around drupal core git interaction, which consider merges and also other aspects on git interaction.

Updated issue summary.

Ok, so what are the todo's to get this done? This has been inactive for almost a year.

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

This came out of the original Drupal 8 initiatives. We where using sandboxes as a place to experiment and prep and collaborate around merge requests. There was no "merge" process at the time only patch acceptance so the idea here was to come up with a way someone making a complex change with multiple commits could get those merged instead of just committing a patch.

In a lot of ways this is resolved with issue forks, I wonder if there's a similar problem with squash commits vs preserving the history but I haven't run into the problem yet.

Think this should be re-open?

Yes, probably replacing _core sandboxes_ with _issue forks_ leads us to the same place.

Currently for core, it seems like almost all issue fork merges are squashed into one commit.
That is definitely an improvement in git history readability comparing it to sometimes full merge and sometimes a squashed commit.

To be fair that mainly happened at the start of the git transition.

As #8 highlights, the main thing is that as a community we are not that good creating patchsets.
That is maybe a consequence of the original restriction of one patch we use to have, that was dragged on into the git era.

Even if we do not move to the patchset model, always squashing is the next best thing, since drupal community is good in issue granularity.

It would be amazing if we move into creating relevant patchsets instead.
Again, by patchset it is not about patches, it is a bout a set of commits that explain the changes, instead of random incremental changes. because they can help to understand better the changes, and even inspect the git history after the fact. E.g. when changing a given file, one may ask what has happened on this set of lines before, and git blame can point to the right commits, and from there more related commits can be found, which can help in the actual new development of the change being worked on.

Moving back to active then.