Topic on Talk:GitLab consultation

Workflow report: force-push

12
Lucas Werkmeister (WMDE) (talkcontribs)

I tried to replicate a Gerrit-like workflow, where you force-push new versions of a single-commit change, in mediawiki/core!5 on the test instance. Since the test instance might go away, I’ll copy/summarize my experience here. (Also, I didn’t yet know how comment threading in GitLab works, so you can watch me figure that out in real time over there ^^)

I like that there is a “compare with previous version” link, like a PS(n-1)..PS(n) diff in Gerrit, which is something that GitHub doesn’t have, as far as I’m aware. To me, this means that a force-push-based workflow is at least not completely unfeasible.

That said:

  • I don’t see any way in the UI to compare a larger range of pushes, say PS2..PS5 (supposing that PS2 was the last patch set you looked at). In fact, so far I haven’t even found a way to do this by hand-editing the URL.
  • The commit message is not considered part of the commit (unlike the COMMIT_MSG “file” in Gerrit), so the second force-push, where I removed a Change-Id from the commit message, shows up as “no changes”.

Overall, my impression is that force-pushing is doable, but likely not the best way to go, or not how GitLab “wants” you to do it. I’d like to try out other workflows as well, e. g. a “squash everything on merge” one.

Adamw (talkcontribs)

This is a helpful investigation, good idea!

My understanding is that rewriting git history is not recommended anywhere outside of the Gerrit workflow. Of course, rewriting is fine locally but as soon as the changes are pushed to a public repo and potentially downloaded by others, rewriting causes problems. I agree that if we were to switch to Gitlab, we should also change our workflows to the natively supported ones rather than try to make our old workflow fit the new system.

Skizzerz (talkcontribs)

Pushing each revision as a new commit, while squashing everything on merge, is a replication of the Gerrit workflow:

  • Gerrit patch sets = GitLab commits
  • Merging a Gerrit patch = Squash merging a GitLab pull request

In both cases, you have the ability to review across "patch sets." No force-pushing is needed, and it's in my opinion a good thing to get rid of it as rewriting history is generally a Bad Idea™ in git, and the requirement to force-push is one of the primary reasons I hate the Gerrit workflow.

Nikerabbit (talkcontribs)

I have never force-pushed with Gerrit. What are you talking about?

Adamw (talkcontribs)

Force-push is the Gitlab equivalent of pushing amended patches for review in Gerrit, but yes to be most accurate we can call the Gerrit workflow "rewriting history".

Nikerabbit (talkcontribs)

Even the merge request workflow is rewriting the history if you use the squash or rebase option.

TCipriani (WMF) (talkcontribs)

The comment that the commit message is not a part of the merge-request is a good note.

I don’t see any way in the UI to compare a larger range of pushes, say PS2..PS5 (supposing that PS2 was the last patch set you looked at). In fact, so far I haven’t even found a way to do this by hand-editing the URL.

FWIW, I have that the changes tab has versions drop downs: https://gitlab-test.wmcloud.org/mediawiki/core/-/merge_requests/5/diffs?diff_id=19&start_sha=8c643bedd7f9e191c8041d3eee682075b4803040

Another example is a https://gitlab-test.wmcloud.org/release-engineering/tools-release/-/merge_requests/2/diffs?diff_id=11 where I added some code review to the mix.

I liked that the comparison between mainline and each revision retained comments on specific code lines even after a force-push.

Nikerabbit (talkcontribs)

For me as a developer, this workflow question is the most important one.

I been playing around with the GitLab test instance today. I really like it... up until to the point I tried to get actual development done.

It is not practical to properly chunk out big changes properly for review with GitLab without history rewriting (i.e. force push). My requirements are:

  • chaining dependent changes together
  • independent iteration of those changes based on code review
  • approving and preferably merging changes independently

Gerrit deals with the problem by having append-only patchset versions and nice way to link them together (no force-push needed).

GitLab seems to primarily support doing it at merge-time. The UI has very limited support for it (only squash and rebase or merge commit). Full power of git rebase -i is needed. If we only allow history rewriting during merge-time, we have multiple issues: the person doing the merge now has the burden to rewrite the history, which in Gerrit is done by the author (except trivial rebases), and they must be able to push directly to the target branch (this would likely be disabled for master branch) and thirdly, the worst thing in my opinion, the result would not be reviewed. All this goes away if we allow history rewriting and force-push updates to the merge request (with sufficient UI support so that reviewers can do their job).

Things get even worse if we also want to independently merge the changes. This means they have to be split into multiple merge requests. I found no support for rebasing merge request chains. In my testing you either have to work backwards and merge latest merge request to earlier one first (frankly, nobody does that), or doing manual force-push for each merge request. Gerrit provides a handy button that does it for most of the simple cases. When you merge a first merge request from a chain in GitLab, the UI gives up. You can change the merge target manually (away from the now [hopefully] non-existing target branch) in the UI, but then, if you squashed or rebased the first merge request, it forces you do do a manual rebase and and force-push to update the subsequent merge requests.

GitLab seems a good solution for work that produces reasonable sized merge requests. However, the suitability of GitLab for me depends on its support for non-trivial history rewriting, which is required to develop and review work that consists of chained changes. Moreover, the only way to do that in GitLab is force-push to a merge request. So the suitability of GitLab essentially hinges on its support for force-push type of workflow.

Lack of a good support for such workflow would, in my opinion, lead to:

  • messy histories with irrelevant and untestable commits, and/or
  • slower development and code review burnout due to difficult to review mega-changes, and/or
  • slower development due to having to wait for author or code-reviewer to complete previous steps before being able to proceed.
Nikerabbit (talkcontribs)

As an update to my previous comment, the force-push workflow does look promising to me. Discussions and comments do not disappear as if they did not exist – they are just marked that they are based on an outdated version of the merge request. I kind of even like this more than Gerrit, where, if you need to iterate, it's unclear whether to continue the discussion in the old patchset, or start a new one in a new patchset. In GitLab you would logically continue the existing discussion until it is marked as resolved. There is even an option to require resolving all discussions before merging. Also the navigation through issues is better than in Gerrit, imho.

As pointed out above, there is an UI to compare different versions of merge requests. I didn't use it as much, so it is not clear whether it is as intuitive and powerful as Gerrit's version. For example, I am not sure yet whether it shows what differences are due to a rebase. I need to explore this more.

I currently think that I could adopt my workflow to GitLab, but I would really miss one feature from gerrit: the rebase button with an option to choose a target. Without it, working on chained merge requests is cumbersome.

Tim Starling (talkcontribs)

I prefer chronological work branches. I think "commit --amend" is an awkward workflow because it leaves you without a local record of your change history. Storing a history of changes seems like a core feature of a VCS, but Gerrit seeks to emulate this with server-side comparison across patchsets. So I'd rather see forced pushes used only for rebases and niche applications rather than importing the complete Gerrit workflow into GitLab.

It's true that instead of having two levels of prospective change management (patchsets and topic branches) we will have one: potentially enormous merge requests of feature branches. I don't think that's infeasible. Simplifying the model in this way is what developers were asking for in the developer satisfaction survey.

AKosiaris (WMF) (talkcontribs)

For what is worth, I 've mostly given up on chronological work branches. I just commit early and often and then very liberally and aggressively edit git history (mostly with interactive git rebase and heavy use squash/fixup/ammend) to produce a set of series of commits that just makes sense (to me) at least as logical unit. Then push for review.


One thing that I would need force push for would be if I realized I got some typo somewhere after creating the MR or realizing I 've made some bad decision when grouping the commits. Not the end of the world if I couldn't do it, but it would be nice to be able to fix them nonethless.


As far as topic branches go, I think that Gerrit topic branches can be in some cases (not sure how many) replaced by gitlab labels (at least I managed to do so in my limited test and for the workflow I was testing).

SSastry (WMF) (talkcontribs)

Pre-gerrit, I was strongly averse to the use of --amend and non-chronological commit history, but gerrit effectively changed my habit on that (once you start using --amend to fix problems based on review, you start using rebase more heavily if you a chain of gerrit patches and reordering commits now only feels normal and natural).

So, in the gerrit workflow, it makes sense, but I imagine once we transition to gitlab, we are more likely to move back to chronological commits (assuming a chain of commits in a pull-request are squashed pre-merge). But, anyway, I imagine we'll have to reconfigure our workflow habits in a gitlab world.

Reply to "Workflow report: force-push"