Analytics/Differential

From Wikitech
Jump to navigation Jump to search

In June 2017 we started using Phabricator's git management solution, Differential, with the Wikistats 2 repository. We chose it as a compromise to use a modern Github-style repository, avoiding gerrit, while at the same time hosting the project in a solution owned by the Foundation. After a trial of nearly four months, the Analytics team has decided to discontinue its use of Differential and to switch to gerrit as of the second quarter of 2017.

Issues

An initial entry barrier to the project

There were three options considered to host the WIkistats 2 project: Github, Gerrit, and Differential. We began the prototyping phase of the project working out of a repository in Github, and since we were used to its asynchronous, more forgiving with errors, approach to git, the consensus in the Wikistats team was to use something similar to Github but hosted by the Foundation. With that motivation, Differential was the way to go. Differential requires the use of a git wrapper called Arcanist, which unlike other wrappers like git-review or the multiple graphic user interfaces for git, is not transparent about the underlying git commands it runs when using it. The installation of Arc is completely manual, and making it work requires the additional step of authenticating with the user's Phabricator account. Giving how bothersome it was for the team to set up and use Arc, it is very likely that someone from the community or another team who might want to contribute to Wikistats will be instantly discouraged to do so because of Arc.

Confusing approach to git

Arcanist does not substitute git's functionality completely, focusing on the code review and pushing remotely aspects thereof. Its three most important commands (and the ones we've used exclusively) are:

arc diff [branch]
This command will push current changes to the remote repository and open a new Revision (code review), whose details we're offered to enter in a nano editor when you run the command. It uses your current branch as a reference, but it is not equivalent to running git push origin [branch], as the code review will not be updated with the new changes when pushing commits to the remote branch.
arc patch [revision number]
We use this code to check out the changes proposed in a Differential code review. It doesn't work, however, like git checkout, as instead of setting the local repository's HEAD to the remote branch's head commit, it will download the changes in the form of a diff and will try to apply that diff to the current HEAD, often giving confusing errors like "patch could not be applied", and leaving the operation half done.
arc land
Merges the change to the main branch and closes the revision, rebasing if necessary

As opposed to most online git interfaces, merging changes to the main branch is not a possibility from the code review page. The only way of merging changes is with the command line.

Insufficient documentation

The team assumes there has to be a reason to use arcanist instead of plain old git, but it's not documented anywhere in its User Guide. The User Guide covers the arc diff command to some extent, but the user has to find out on their own how to check out changes or how to merge to the remote repository, and the technical documentation is a big list of commands and plugins that aren't very helpful.

Prone to error behaviour (dark patterns)
  • Every commit pushed with arc seems to carry some sort of reference, which is required by Differential. If the user tries to push a commit directly through git to a remote branch that is undergoing a code review, differential will auto-close the revision in an irreversible way, Nowhere in the documentation is explained what is the logic behind this.
  • If the revision was auto-closed and the user wants to open a new one with the unmerged changes from the former, arcanist will reject it, saying there is already a revision for that change.
  • If there are unstaged changes in the local working directory, arcanist won't allow the user to proceed with the diff command, forcing them to either commit, reset or stash.
  • There have been instances of code removed when merging using arc land with one revision depending on another.
  • Once a revision has been accepted, its state on the Differential dashboard won't change from "Waiting on authors", regardless of any changes to its code.

Other cons

  • Unlike Gerrit, there seems to be no way of comparing changes between different commits of the same proposed change. This makes it difficult to follow the code review process.
  • When commenting, you can only select the whole line. There's no way to select just a variable, or an expression, etc. This quite useful feature is missing.
  • Also, comments referring to older versions of the code, are displayed in new versions of the code and are totally confusing. Because they might refer to code that isn't there any more, or that is in a completely different form.
  • There's a lot of scrolling required to go over the code. Before the code starts, there's a table of contents, that lets you skip directly to a file, but once you are in the middle of the code, you have to scroll a lot.
  • There's no "my changes" page, where a developer can see all code reviews assigned to them. This is a huge plus for Gerrit.

Advantages

One advantage of Differential is the comment editor. Comments accept formats, uploads, pings, etc. which helps a lot when expressing issues.

Next steps

task T177288

Move to Gerrit

After some discussion in the team we reached the consensus that we're moving the Wikistats 2 project into Gerrit. Our git methodology follows that of git flow, meaning we have a master branch from which we do releases and deployments, a develop branch, which holds the latest stable build and all code reviews are merged into, and then feature branches that correspond to each Phabricator task. We'd like to keep this workflow more or less intact, so the develop branch should be treated as the main one.

The gerrit repo should be mirrored for read-use only in Github.

Set up CI

RIght now each time a commit is pushed to a Differential revision, a jenkins job is started to run our tests. This job should be triggered from Gerrit.

Update documentation

There is documentation on contributing to and deploying Wikistats in wikitech which will be obsolete once this change is executed. All this documentation will be archived in this wiki page and should be updated with the new procedures when we move to Gerrit.