Code Review & MergeBot workflow challenges

[jesse_unity624]

We’re evaluating UVCS for our studio and so far we’ve been experiencing a bit of friction with the code review & branch merging process. Some of these things are minor annoyances, and others are things team members (especially less-technical ones) are likely to get wrong more often than not. We’re wondering if there are some best practices out there, or work in progress on improving the tooling.

Some of what we’ve run into so far:

• In the desktop application you can’t make a review for an entire branch from the Branch Explorer, instead you have to switch to the Branches tab. In general the context menu on a branch label in the Branch Explorer is very misleading since it rarely acts upon the branch itself (our team has ended up creating many single-changeset reviews this way).
• Code reviews don’t describe where they’ll be merging to. In other words, they’re truly a branch code review, not the equivalent of a merge review in Perforce, or a pull request in git.
• In general, the “I’m ready to merge this, can people please check out if it’s ok” workflow seems under-supported.
• There’s no rebase, so the recommended flow is merge from parent to child. But once you do that, the default view in a code review shows all the file changes from the parent branch in the review, confusing reviewers about what’s changed in the branch and what they actually need to review, adding friction.
• There’s nothing showing you whether your branch is ready to merge, or out of date, or will have merge conflicts, forcing you to leave the review in order to get these answers, adding friciton.
• You can’t assign groups of individuals to a review, so automatically assigning a Developers team, or a UI team to a review doesn’t actually help you get the content reviewed.
• If you haven’t been individually invited to a review you aren’t allowed to mark the review as reviewed. This turns the code review approval process from a passive team culture process into an active “I need to go find an individual person who will review my code and invite them” step - it adds friction.
• Mergebots actually make the workflow more manual than not, adding friction. Once you ARE ready to merge things, if you’re using a merge bot the interface is unintuitive and error prone. Where all other code review tools I’ve used have a 1-step process which is the equivalent of a “Complete review” or “merge pull request”, or even a “merge once review has enough approvals” etc. Plastic has a multi-step process:

1. Leave the code review
2. Go to the branches view
3. Select the branch the review was for
4. Click the show properties button
5. Click Apply Attributes
6. Select the appropriate attribute name from the list (“status” according to the tutorials)
7. Select the appropriate value from the list (“resolved” according to the tutorials)
8. Then the “automation” should handle it for you

What are other peoples experiences with this tool in a multi-discipline team? Are we just using it wrong? Is there a better way to get work reviewed and into a main/develop/trunk branch?

[SRombautsU]

Hi,

Just answering quickly this one, as it might be a bit misleading:

In the desktop application you can’t make a review for an entire branch from the Branch Explorer

You have to be careful to right click on the branch to get the proper menu.

Compare the Changeset context menu:

Versus the Branch context menu:

I am always creating code reviews (and basically everything) from the Branch Explorer.

The rest of the workflow that you describe isn’t mandatory, it’s what the original dev team from Codice Software selected for themselves, promoting one particular form of DevOps.

Coming from intensive use of Github, I certainly agree with most of your statements, but I want to emphasis that most of the behavior that you describe aren’t mandatory:

• For instance, you can add yourself to any review, and approve it like in Github, depending on your repository config and user privileges.

Cheers,
Sébastien

[jesse_unity624]

Thanks Sébastien, it’s good to know you can create a branch review from the branch explorer. It looks like our confusion came from the inconsistent behavior of the branch labels. If you right-click on anything else in that view the behavior is “select the thing you clicked on, then open its context menu”, but for branch labels the behavior is just “open the context menu for the previously selected item”… this leads to weird experiences, like right-clicking on a branch label and getting the context menu for a selected changeset in a different branch:

On the rest, yes, it’s a workflow, not mandatory - which I do appreciate. I’m just looking for some suggestions on improved workflows and how to reduce the friction here… I’m on a team with a feature-branch strategy, prefers gated check-ins, requires peer review, etc.

There seems to be a significant number of choices which appear to have been made to simplify tasks which actually just reduce choice. Like the lack of control between a code review and a merge - code reviews are dead simple to make (1 click in a context menu), but because they’re decoupled from the merge process there’s no way to have someone review how the merge would go (say if you want to bring a bug fix into 2 branches).

On the flip side, a lot of places where other platforms provide quality of life capabilities that aren’t present here. Like if there’s any way to get closer to the 1-step close-and-merge processes from other VCS platforms instead of the 8-step process with the mergebots. Do people just recommend not using the mergebots with a code review flow? Or how you can add yourself to a code review, but until you do so you can’t give a thumbs-up - other platforms just automatically add you to the review if you try to give a thumbs-up, but plastic makes it extra steps with no error messaging guiding a user.