Page MenuHomePhabricator

Proof of concept of code review in Phabricator
Closed, ResolvedPublic

Description

We need to build a proof of concept in Labs showing how Wikimedia code review would work in Phabricator.

The goal of this PoC is to help polishing details for T18: Plan to migrate code review from Gerrit to Phabricator and to ease community testing and feedack.

Desired deadline: December 2014.

Feature set (early draft)

  • Dedicated Labs instance with daily backups.
  • Several repositories imported
  • Process for creating new repositories with some examples (i.e. FOSS Outreach Program for Women projects?) No migration will be provided, just git clone.
  • Browse repositories
  • Subscribe to repositories
  • Become an owner (maintainer) of a repository
  • Inherit policies in a tree
  • Install Arcanist in Ubuntu, Fedora, MacOSX, Windows.
  • Instructions to clone a repository and prepare a patch.
  • Example of pre-upload linting, and possibility to bypass.
  • Possibility to be notified as repository maintainer, subscriber, or by task.
  • Comment, vote, reject, approve, abandon, merge.
  • Dependent patches, obsolete dependency.
  • Display whether a patch is mergeable or not.
  • Testing that non-maintainers have no way to bypass code review to get their code merged.
  • List of open diffs waiting for review.
  • Marking diffs as work-in-progress, or equivalent to "-1", so they can be "ignored".

(Continuous Integration PoC? Harbormaster? To be decided.)
(Specify which of these options could be performed via mobile)
(Any interesting Herald actions that we could include?)

Related Objects

StatusAssignedTask
Resolved Dzahn
ResolvedCmjohnson
Resolved Dzahn
Resolveddemon
Resolveddemon
ResolvedDanny_B
ResolvedPaladox
ResolvedPaladox
ResolvedNemo_bis
Resolveddemon
ResolvedPaladox
ResolvedKrenair
Resolvedmmodell
InvalidNone
DeclinedNone
Resolveddemon
InvalidNone
InvalidNone
ResolvedQgil
DeclinedNone
DuplicateNone
Resolvedgreg
Resolvedgreg
Resolveddemon
Resolvedgreg
DeclinedNone

Event Timeline

Qgil created this task.Oct 7 2014, 3:30 PM
Qgil updated the task description. (Show Details)
Qgil raised the priority of this task from to Normal.
Qgil claimed this task.
Qgil added a project: Gerrit-Migration.
Qgil changed Security from none to None.
Qgil added a subscriber: Qgil.
Qgil updated the task description. (Show Details)Oct 12 2014, 9:07 PM

I have added a first draft of the feature set of this proof of concept. Feedback?

Qgil lowered the priority of this task from Normal to Low.Oct 28 2014, 2:24 PM
Qgil removed Qgil as the assignee of this task.Nov 3 2014, 12:52 PM
In T18#1061505, @Qgil wrote:

@chasemp, @mmodell, @Aklapper, and myself had a meeting about Phabricator misc topics yesterday. We agreed that if we want to work on T560: Proof of concept of code review in Phabricator in a hackathon. Wikimania is more suitable than Lyon because of dates and travel preferences.

Note that the four of us also tend to think that we can just enable Differential for whoever wants to try it out (probably starting in phab-01 to define the workflow and let people play). We have the impression that only by seeing it in action people will realize the differences in the user experience beyond the UI. We agreed that you don't even need a hackathon or an online sprint of sorts to work on this, although we do see the point of putting aside the daily routine to work as a team on this goal during a couple of days, face to face.

Considering that Harbormaster is in a rough alpha state, I would be a little hesitant in using it to provide continuous integration but if this is solely as a demonstration of Phabricator's repo management then I say we should go for it. Harbormaster shouldn't be in alpha forever and then we may (very far off) think about it replacing Jenkins and Zuul.

There are sane ways to hook a Jenkins type thing directly into Differential. That is what people have been doing for awhile, and since before Harbormaster was a thing. Not suggesting this is what we are doing but it's not a hard deterrent either.

mmodell added a comment.EditedFeb 25 2015, 5:23 AM

@Negative24: I don't think demonstrating phabricator's repo management is the primary goal. The goal is to get people to try out arcanist+differential for code review.

They all fit in line though. If we have no plans to look at Harbormaster than so be it. I was just mentioning that a full Phabricator integration would be beneficial if we are looking at a model that implements it that way.

jayvdb added a subscriber: jayvdb.Mar 2 2015, 10:10 PM
Qgil added a comment.Mar 6 2015, 9:17 AM

Never surrender... :)

On Fri, Feb 20, 2015 at 5:35 PM, Chad Horohoe wrote in an email:

Step 1 should be doing projects that don't even need Gerrit or Jenkins. Then we can focus on the Arc-Differential bits right without any other moving pieces.

What about a demo-able project for the Wikimedia-Hackathon-2015 consisting Pywikibot code review in Phabricator? Could be a test in phab-01 or the real deal in production, depending on how good is the implementation and how on board the Pywikibot folks are.

If @mmodell (Wikimedia Phabricator maintainer) and @jayvdb (Pywikibot maintainer) are also in, I don't think we require anything else to try this out in Lyon. Other Gerrit-Migration interested parties will be either in Lyon or available online, busy with other things but probably able to answer and help on specific bits during the event. It is very likely that more Pywikibot folks will be at the event. It is going to be a very good environment for ad-hoc technical & social discussions and decisions in the Phabricator and Pywikibot sides.

Also, if we define what "projects that don't even need Gerrit or Jenkins" means, we might get other projects during the weekend from the pool of bots, Tool Labs, etc, that will be at the event.

No mater at which point we leave the work at the end of the hackathon, we could continue polishing after the event, opening the door to all these non-Jenkins projects, and starting to build a critical mass of projects using Differential for code review.

@Qgil What parts of code review are you planning on having Pywikibot handle?

Qgil added a comment.EditedMar 27 2015, 1:49 PM

@Negative24, I'm not sure I understand. I'm proposing to move Pywikibot code review from Gerrit to Differential here in Wikimedia Phabricator. I don't have a strong opinion about which repos should we start with, other than full buy-in from their maintainers and OK from the Release Engineering team.

In other news, I have created T94167: Opt-in Differential for projects needing code review but not Jenkins/Zuul for continuous integration specifically for the Wikimedia Hackathon plan.

@Qgil Sorry, the way you phrased it sounded like you wanted to implement code-review run by #pwikibot.

I won't be in Lyon (you know), but I'm interested in pioneering Differential with some of my Labs tools!

This comment was removed by Aklapper.
revi added a subscriber: revi.May 28 2015, 12:33 PM
Qgil added a comment.Jul 3 2015, 10:26 AM

Please confirm and promote this activity by assigning it to its owner, listing it or scheduling it at the Hackathon wiki page and by placing it in the right column at #Wikimania-Hackathon-2015. Thank you!

Differential is available in an opt-in basis. RelEng is using it for Scap work (see: https://phabricator.wikimedia.org/differential/query/Bl4g_4o5nAT8/ )

greg closed this task as Resolved.Dec 1 2015, 4:41 PM
greg claimed this task.
greg added a subscriber: greg.

This task isn't really valid anymore. If teams want to start using Differential, they can, just ask us. There are caveats (you don't get all of our Continuous Integration yet, change aren't automatically announced on IRC for you, etc).

Scap is being developed solely in Differential as of now, so I guess that's a "proof of concept code review in phab".

Restricted Application added a project: User-greg. · View Herald TranscriptDec 1 2015, 4:41 PM
demon moved this task from To Triage to Done/Archive on the Gerrit-Migration board.Dec 1 2015, 6:46 PM
Qgil awarded a token.Dec 1 2015, 7:59 PM