Page MenuHomePhabricator

[RfC]: Migrate code review / management from Gerrit to Phabricator
Closed, DeclinedPublic

Tokens
"Heartbreak" token, awarded by mmodell."Heartbreak" token, awarded by Niharika."Heartbreak" token, awarded by MGChecker."Heartbreak" token, awarded by JeanFred."Mountain of Wealth" token, awarded by tom29739."Yellow Medal" token, awarded by Negative24."Like" token, awarded by Qgil."Love" token, awarded by Luke081515."Yellow Medal" token, awarded by Jdforrester-WMF."Like" token, awarded by Aklapper."Dislike" token, awarded by Ricordisamoa."Love" token, awarded by RobLa-WMF.
Assigned To
None
Authored By
demon, Nov 30 2015

Description

This is an RFC to migrate code-review from Gerrit to Phabricator's Differential.

Full RFC on mediawiki.org:
https://www.mediawiki.org/wiki/Requests_for_comment/Migrate_code_review_and_management_to_Phabricator_from_Gerrit

Plan as it relates to WMF RelEng's quarterly goals:
https://www.mediawiki.org/wiki/Wikimedia_Release_Engineering_Team/Project/Differential_Migration
(just a team/goal planning document, but it helps get an idea of the steps/overall plan)

Related Objects

Mentioned In
T210820: Migrate Differential repo rWIEG back to Gerrit wikimedia/iegreview.git
T191182: Stop using Differential for code review
T165208: Replace Gerrit with something usable
T190139: create phab repo extension-AddPersonalUrls
T613: Arcanist security review (before being used in WMF deployments)
T187957: Set up Github -> Diffusion -> Deployment workflow for Research landing page
T141158: [Differential] Add Differential support to CI jobs
T141159: [Differential] Remove Gerrit support from repo and jobs
T130421: Phase 3 repository migrations to Differential
T122979: Write script to migrate open changes from Gerrit to Differential by repository
T122834: Move the OOUI library's development from gerrit to differential
T617: Provide static dump of Gerrit
T115624: Have Phabricator take over replication to Github
T131418: Migrate mediawiki-config to Differential
T131419: Migrate mediawiki-vagrant to Differential
T132173: Migrate wikimedia-wikimania-scholarships to Differential
T134434: Migrate mwdumper to Differential
T167: Align basics of current Gerrit code-review process with Differential
T131955: Cross-repository gating of changes pre-merge in Differential
T92493: Train L10n-bot to work with repositories hosted in Wikimedia Phabricator (Diffusion)
T207: Update Code Review related documentation on wiki pages from Gerrit to Differential
T123081: Update Commit Message Guidelines for Differential
T187149: Delete all Phabricator git repos that haven't been referenced / aren't used.
Z425: ArchCom open discussion
T143969: Unable to mirror repository from git.legoktm.com into diffusion
E152: RFC Meeting: Architecture office hour (2016-03-30, #wikimedia-office)
T118753: Make MetricsGrimoire/korma support gathering Code Review statistics from Phabricator's Differential
T128372: Document use of Owners in Phabricator and advertise it
T125865: Assign RFCs to ArchCom shepherds
T115844: Have a second Gerrit (Code Review backlog) cleanup day
T584: Define main tasks (epics) for code review in Phabricator
T114311: [keyresult] Code review RFC (Differential) - Write, publish, publicize, and respond to/incorporate feedback.
T114320: Code-review migration to Differential status/discussion
Mentioned Here
T191182: Stop using Differential for code review
T156120: Update gerrit to 2.14.6
T165208: Replace Gerrit with something usable
T130420: Phase 2 repository migrations to Differential
T131955: Cross-repository gating of changes pre-merge in Differential
T130949: Spec out needed glue for Differential to Gearman to Nodepool
E146: RFC Meeting: triage! (2016-03-02, #wikimedia-office)
T31: [keyresult] Connect Differential code review with continuous integration
T114320: Code-review migration to Differential status/discussion
D46: Initial Debian packaging
D51: Redirect gerrit repo URIs to diffusion callsigns.
T110607: redirect gerrit repo paths to diffusion callsigns
T114419: Event on "Make code review not suck"

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 26 2016, 5:24 PM
labster added a subscriber: labster.May 7 2016, 7:38 AM
Luke081515 renamed this task from [RfC]: Migrate code review / management to Phabricator from Gerrit to [RfC]: Migrate code review / management from Gerrit to Phabricator.Jun 29 2016, 11:49 PM
RobLa-WMF removed RobLa-WMF as the assignee of this task.Aug 17 2016, 7:32 PM

Removing myself as assignee to (hopefully) reduce confusion about what the "shepherd" role is about (TechCom now uses column on the TechCom-Has-shepherd board to communicate shepherd information)

Qgil moved this task from Backlog to Team radar on the Developer-Advocacy board.
daniel added a project: TechCom-RFC.
daniel moved this task from Under discussion to (unused) on the TechCom-RFC board.Nov 16 2016, 6:44 PM
daniel moved this task from (unused) to Backlog on the TechCom-RFC board.Feb 8 2017, 9:35 PM
jayvdb added a subscriber: jayvdb.May 13 2017, 8:35 AM
EddieGP removed a subscriber: mmodell.May 13 2017, 2:42 PM
Dvorapa removed a subscriber: Dvorapa.
hashar removed a subscriber: hashar.Dec 22 2017, 2:04 PM
daniel added a subscriber: daniel.EditedJan 9 2018, 8:00 PM

Is there still commitment to doing this? Is this RFC looking for approval?

@daniel from what i understand it's on hold.

Paladox lowered the priority of this task from Normal to Lowest.Jan 9 2018, 8:09 PM

Changing to lowest as normal would indicate it will be happening soon.

EddieGP added a subscriber: mmodell.Jan 9 2018, 8:34 PM

Is there still commitment to doing this? Is this RFC looking for approval?

No:

@EddieGP: this is indeed stalled and currently we don't see any way forward. The status quo (gerrit) will likely remain for the foreseeable future.

By now, I'm not sure whether it's really "we can't commit time to this currently" (=stalled) or "we decided to stay on gerrit for now and are not sure if that's ever going to change" (= declined) though.

greg added a comment.Jan 9 2018, 8:35 PM

We're (RelEng) going to talk about this at our team offsite (post DevSummit/All hands, the week of the 29th). I'll give a summary of that discussion here.

This comment was removed by mmodell.
HappyDog added a comment.EditedJan 10 2018, 1:48 AM

Is there still commitment to doing this? Is this RFC looking for approval?

No:

This really needs reviewing, as Gerrit is a huge barrier to entry for new developers. See the summary for T165208 (marked as a duplicate of this ticket) for just some of the problems that discourage contributions.

Seriously, so long as Gerrit is being used, you are actively discouraging participation.

It's not as big of a a barrier to contribution as you think. Generally, when I want to commit something on a project hosted by WMF, I push a commit to somewhere else like Github, and then ask someone else who understands Gerrit to merge it. Usually, this is the extension developer, and from there they can either do git remote add or download a patch file.

Paladox added a comment.EditedJan 10 2018, 7:24 AM

Is there still commitment to doing this? Is this RFC looking for approval?

No:

This really needs reviewing, as Gerrit is a huge barrier to entry for new developers. See the summary for T165208 (marked as a duplicate of this ticket) for just some of the problems that discourage contributions.

Seriously, so long as Gerrit is being used, you are actively discouraging participation.

@HappyDog Gerrit is a lot easier then differential as gerrit dosent force you to use a tool to contribute. Gerrit includes a inline editor where as differential dosen’t.

If you think gerrit is hard for new users, could you file a task in Gerrit please with an explementation on why it’s hard and if you have any ideas on how it could be resolved. I can then upstream the task.

@HappyDog is it the UI that’shard? If so gerrit’s is getting a new ui

HappyDog added a comment.EditedJan 10 2018, 9:22 AM

It's not as big of a a barrier to contribution as you think. Generally, when I want to commit something on a project hosted by WMF, I push a commit to somewhere else like Github, and then ask someone else who understands Gerrit to merge it. Usually, this is the extension developer, and from there they can either do git remote add or download a patch file.

And how do you handle the review process. Take a look at my description in T165208 - even locating the button to add a comment is difficult!

@HappyDog is it the UI that’shard? If so gerrit’s is getting a new ui

I think it's just the UI, but because the UI is so bad it is hard to tell whether there are also underlying issues. I expect not, and that it is just a UI problem.

What is the timeline on the new UI? Has anyone seen any mock-ups or work-in-progress yet? Is it likely to solve the usability issues, or is it just a cosmetic dusting on top of what is currently there?

I remember taking a look at a 'new UI' back in May (as linked to from this comment, though the link is now dead), and it seemed like an incremental step that didn't address a lot of the fundamental UI issues, but perhaps things have developed since then.

Please move "me too" / "UI bad" / "UI good" / "I don't like XYZ" / "+1!"s on Gerrit or Phabricator to the talk page on-wiki instead of posting them here. Thanks.

demon added a comment.Jan 12 2018, 5:16 PM

What is the timeline on the new UI? Has anyone seen any mock-ups or work-in-progress yet? Is it likely to solve the usability issues, or is it just a cosmetic dusting on top of what is currently there?

  1. It's opt-in starting with gerrit 2.14.x (upgrading soon, cf T156120). It becomes the default with 2.15.x.
  2. Of course we've seen mockups and a WIP: it's being developed upstream and is already in use there
  3. I think it solves a lot of usability issues (it wasn't just a bunch of CSS fixes to an existing framework....)

But then again, I find it hard to take you seriously when you dismiss the new UI as "not massively full of whelm." Concrete issues ("this thing is hard to find/do", "when I try to X, I can't") are solvable and upstream-able. Vague complaints like "UI/UX is ugly, scrap it, Oh btw ever heard of Github?" are completely useless (and borderline condescending).

I remember taking a look at a 'new UI' back in May (as linked to from this comment, though the link is now dead), and it seemed like an incremental step that didn't address a lot of the fundamental UI issues, but perhaps things have developed since then.

Well 6 months ago it was so incomplete as to be basically unusable. So I think you'll want to give it a fresh trial run once we upgrade--then see where we stand. We'll want to reassess any UI/UX related bugs that have already been filed. Some will be solved outright--some may be differently broken now.

HappyDog added a comment.EditedJan 13 2018, 2:20 PM

Apologies if you found my comments condescending. That was not the intention.

I find it hard to take you seriously when you dismiss the new UI as "not massively full of whelm."

I'm not really sure how this differs from your description of the UI as "basically unusable", aside from being phrased in a more colloquial fashion (which, in a global forum such as this, was probably a mistake). However, we seem to agree on the basic point, that what was being proposed at that time was not fit for purpose.

Concrete issues ... are solvable ... Vague complaints like "UI/UX is ugly" are completely useless.

I did include some concrete issues, but really this is not the forum for getting into the detail. The issues are systemic and require an upstream UI overhaul, not a bunch of individual requests to 'move button x' or 'improve icon y'.

The 'new UI' that was linked to 6 months ago was - as you say - unusable, which really didn't bode well. I will be interested in seeing the finished implementation to see whether it has made a difference. If so, maybe the fix to T165208 would simply be to upgrade, which would be great!

As for the current version of Gerrit being ugly - well, that's subjective of course, but I would question the taste of anyone who picked it out of a beauty contest between Github, Phabricator and even - why not - Bugzilla. :-)

@HappyDog this is what it will eventually look like https://docs.google.com/presentation/d/17q-ygGioZi_5DITLyELa8oaOr22e15AHy8cq6XTZ0nY/edit#slide=id.g27f16618ec_0_139

Also this is a rewrite of the ui, it is done in javascript and html. It's based on polymer which youtube uses.

The ui is alot more usable from 2.15+.

As for the current version of Gerrit being ugly - well, that's subjective of course, but I would question the taste of anyone who picked it out of a beauty contest between Github, Phabricator and even - why not - Bugzilla. :-)

In retrospect, "it must not be ugly" is a terrible criteria for software selection :P

We're (RelEng) going to talk about this at our team offsite (post DevSummit/All hands, the week of the 29th). I'll give a summary of that discussion here.

Is there an update from this?

daniel added a comment.Mar 7 2018, 8:57 PM

TechCom wonders what to do with this. Can we drop it from the RFC process? Or close it?

mmodell closed this task as Declined.Mar 7 2018, 9:56 PM


:'(

demon added a comment.Mar 7 2018, 10:03 PM

(was about to close as well, here's what I was writing)

For a variety of reasons, this RfC will not be moving forward as structured. There's no current plans for a migration from Gerrit to Differential. If the project were to spark up again, it would benefit from a new RfC with fresh perspectives

Should we also close tasks from Gerrit-Migration and archives it?

demon added a comment.Mar 7 2018, 10:35 PM

Should we also close tasks from Gerrit-Migration and archives it?

Some of them are worth retaining. The project should be archived yes.

(+ https://www.mediawiki.org/wiki/Requests_for_comment/Migrate_code_review_and_management_to_Phabricator_from_Gerrit , https://www.mediawiki.org/wiki/Wikimedia_Release_Engineering_Team/Project/Differential_Migration , https://www.mediawiki.org/wiki/Phabricator/Migration_Timeline might welcome updates.)

Declined the first. Marked second and third as historical.

IMHO the current state is the worst possible outcome. In addition to what we already had for most (gerrit) or some (github) of our repos, we now have a third place for some others (some tools on toolforge, Scap and a few more host their code on diffusion and do code review on differential).
Especially from a new contributors perspective, this is bad. Depending on which project I want to contribute to I now have to familiarise myself with github, gerrit and/or differential. Personally, when I started contributing here about a year ago, I didn't know about any of those and wouldn't have cared which one to learn. But by now, I had to dig into differential as well as gerrit, because I wanted to contribute to different projects over time.
This is an acceptable state for a (gerrit->differential) transition period. It isn't when the transition was declined. One can argue a lot about whether gerrit or differential fits our purpose better (which was done) - but I hardly can imagine how keeping both isn't much worse than having either of them.
tl;dr: The migration gerrit->differential being declined and gerrit being supposed to stay around, we should consider to shutdown differential and migrate existing projects from there to gerrit.

Not sure whether this is the right place to start that discussion, probably not. Feel free to move it to wherever seems more appropriate.

greg added a comment.Apr 1 2018, 7:39 PM

Not sure whether this is the right place to start that discussion, probably not. Feel free to move it to wherever seems more appropriate.

It is not. This RFC is declined.

It is not. This RFC is declined.

It's at T191182 now. But maybe it should go on some mailing list. Or some wiki discussion page. An answer that includes where to put it instead would've been more helpful.

mmodell rescinded a token.Apr 9 2018, 8:19 AM
mmodell awarded a token.