Page MenuHomePhabricator

Update gerrit submit type for discovery repositories in gerrit
Closed, ResolvedPublic

Description

Currently most (all? haven't checked) discovery repositories are set to the default submit type in gerrit, specifically Merge if necessary. Basically this means gerrit will try to fast forward, and if that fails it will create a merge commit. Alternate submit types, used in a variety of operations/* and integration/* repos, are Rebase if necessary and Fast Forward Only. Both of these options eliminate merge commits and put git log in the order that commits are released, the primary difference is Fast Forward Only requires the user to rebase, while Rebase if necesssary lets gerrit try.

The motivation here was that I, being quite silly, misread a git log and attributed a bug to the position in the log that the commit changing things shows up, instead of at the merge commit when that code was applied. Putting the log in release order will help simplify tracking down bugs in these logs. This will also reduce spam in the logs from merge commits, as they will no longer be necessary.

It's not clear yet if Rebase if necessary or Fast Forward Only is the preferred option. Rebase is the simplest, while fast forward is the most explicit. In the end for most of the discovery repos requiring fast forward is likely overkill.

Event Timeline

hashar subscribed.

The documentations about submit type is at https://gerrit.wikimedia.org/r/Documentation/config-project-config.html#submit-type

Fast forward only would cause a change to require a manual rebase whenever the target branch has advanced. That is the model that was used for operations/puppet and it was a major source of pain. When two people tried to submit a change, one would have to rebase which was a bit of a hell. The idea behind that strategy is that you know your CI has run the tests with the tip of the branch and prevents potential failures that could appear if Gerrit was merging/rebasing behind your back. Our CI already does the tests of the change against the up-to-date tip of the branch so that is strategy has no use case for us.

Rebase if necessary with the option Allow merge content is a good fit for our setup since CI already resolved the merge/rebase and the patch is known to work with the tip of the branch. Compared to Merge if necessary it has the advantage of not introducing extra merge commit, but if it has to rebase, that introduces an extra patchset to the change (which imho is not a big deal).

I guess we ran raise awareness of that feature and change the default? It is currently set to Fast forward only and Allow content merge: false but I am not sure why.

mediawiki/* has Merge if necessary

EBernhardson triaged this task as Medium priority.
EBernhardson moved this task from needs triage to Current work on the Discovery-Search board.

Changed search/ and wikimedia/discovery/ parents to Rebase if necessary with Allow content merges set to True. Reviewed sub-repositories and changed everything to inherit if it wasn't already. Additionally changed mediawiki/extensions/{CirrusSearch,GeoData,WikibaseCirrusSearch}. wikidata/query/rdf was reviewed and is already configured this way.

Looks great, thank you @EBernhardson to have applied the change ;)