Identify Arcanist showstoppers for wikimedians
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Qgil
	Oct 9 2014, 3:22 PM

Description

Arcanist (aka Arc) is the Phabricator tool to upload patches for code review. You can find a reasoning for Arcanist to exist by its creator. Is it missing anything essential for code review in Wikimedia?

There are many tasks related to Arcanist, and many more that might come. However, which ones are the real showstoppers that we either fix in Arcanist or find workarounds elsewhere?

We need to identify real showstoppers and their potential solutions in order to plan the code review migration accordingly (T18).

Related Objects
Search...

View Standalone Graph

This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Status	Assigned	Task
		· · ·
Duplicate	None	T18 Plan to migrate code review from Gerrit to Phabricator
Resolved	greg	T597 Identify Arcanist showstoppers for wikimedians
Invalid	greg	T612 Landing a patch with arc currently will sometimes strip author information
Declined	None	T137 Running `arc diff` twice on the same commit creates another redundant diff within the same differential
Declined	None	T127 Find way to use Differential with plain git (i.e.: without requiring arc)
Declined	greg	T164 Uploading a new Differential Revision does not reset “Accept” states in commit messages
Invalid	• mmodell	T181 Figure out how to configure default arc diff behavior
Resolved	greg	T91422 Phabricator and Arcanist versions must be kept in sync because of occasional breaking api changes between phabricator versions.
Invalid	• mmodell	T99905 You should be able to merge your own patches
Duplicate	None	T112967 Document git-review -> arc mapping.
		· · ·

Event Timeline

Qgil created this task.Oct 9 2014, 3:22 PM

Qgil raised the priority of this task from to High.

Qgil updated the task description. (Show Details)

Qgil added projects: ECT-October-2014, Gerrit-Migration, Wikimedia Phabricator RfC.

Qgil changed Security from none to None.

Qgil removed projects: Wikimedia Phabricator RfC, ECT-October-2014.

Qgil added subscribers: Aklapper, Qgil, greg and 15 others.

Qgil mentioned this in T584: Define main tasks (epics) for code review in Phabricator.Oct 9 2014, 4:28 PM

ArielGlenn subscribed.Oct 9 2014, 4:54 PM

Qgil moved this task from To Triage to Need Discussion on the Gerrit-Migration board.Oct 9 2014, 8:42 PM

Qgil added a subtask: T612: Landing a patch with arc currently will sometimes strip author information.Oct 10 2014, 8:19 AM

Qgil updated the task description. (Show Details)Oct 10 2014, 8:22 AM

Qgil mentioned this in T162: Commit hashes of landed patches need not match the latest ones shown in Differential.Oct 10 2014, 11:48 AM

After going through all the Arc related tasks, I could only find T612 as a potential blocker.

Maybe others read the current open tasks different. If this is your case, please point us to tasks that you consider blockers indeed, or create new ones.

Maybe we find more while working on T560: Proof of concept of code review in Phabricator, but I don't see anything that could stop the work of the PoC itself.

Qgil lowered the priority of this task from High to Medium.Oct 20 2014, 10:35 PM

I'm assuming this means show-stoppers for the POC. Something might be good enough for the POC, but we might not be sure whether it's good enough for everything.

Also, is this meant to include Differential (the actual code-review tool) show-stoppers? If so, it should be renamed, and we should go through T23 again looking for show-stoppers.

For now, I'll add some potential arcanist blockers (open for discussion).

• Mattflaschen-WMF added a subtask: T137: Running `arc diff` twice on the same commit creates another redundant diff within the same differential.Nov 7 2014, 5:11 AM

• Mattflaschen-WMF added a subtask: T127: Find way to use Differential with plain git (i.e.: without requiring arc).

• Mattflaschen-WMF added a subtask: T164: Uploading a new Differential Revision does not reset “Accept” states in commit messages.Nov 7 2014, 5:22 AM

This task focuses on Arcanist alone. It would be interesting to list here all blockers, highlighting blockers for the POC if needed.

T167: Align basics of current Gerrit code-review process with Differential focuses on code review (Differential). T31: [keyresult] Connect Differential code review with continuous integration Focuses on continuous integration.

These three tasks correspond to the three steps: upload, review, and submit. Does this separation make sense?

Qgil updated the task description. (Show Details)Nov 7 2014, 11:13 AM

It's probably reasonable enough. If someone's not sure where to put it, they can use T167: Align basics of current Gerrit code-review process with Differential.

• Mattflaschen-WMF added a subtask: T181: Figure out how to configure default arc diff behavior.Nov 14 2014, 12:06 AM

• Mattflaschen-WMF unsubscribed.

Currently #Code-Review is in the queue of Phabricator subprojects after Project-Management and Gitblit-Deprecate. If someone wants to work on this task, take it and assign the priority accordingly.

Liuxinyu970226 unsubscribed.Nov 27 2014, 8:05 AM

• MZMcBride subscribed.Dec 5 2014, 7:22 PM

JanZerebecki subscribed.Mar 3 2015, 6:26 PM

JanZerebecki added a subtask: T91422: Phabricator and Arcanist versions must be kept in sync because of occasional breaking api changes between phabricator versions..Mar 3 2015, 6:28 PM

epriestley closed subtask T181: Figure out how to configure default arc diff behavior as Resolved.Mar 4 2015, 8:19 AM

• demon reopened subtask T181: Figure out how to configure default arc diff behavior as Open.Mar 4 2015, 8:40 AM

hashar unsubscribed.Mar 4 2015, 9:26 AM

Qgil moved this task from Need Discussion to Tooling on the Gerrit-Migration board.May 21 2015, 8:42 AM

Ricordisamoa subscribed.May 21 2015, 9:16 AM

greg closed subtask T612: Landing a patch with arc currently will sometimes strip author information as Invalid.May 24 2015, 8:43 AM

• mmodell closed subtask T181: Figure out how to configure default arc diff behavior as Invalid.May 26 2015, 10:27 PM

JanZerebecki mentioned this in T23: Identify features Gerrit users would miss in Phabricator.May 27 2015, 12:40 PM

Glaisher subscribed.May 27 2015, 12:45 PM

cscott subscribed.Sep 17 2015, 1:54 PM

cscott added a subtask: T112967: Document git-review -> arc mapping..Sep 17 2015, 8:56 PM

I'm closing this task. There is the Gerrit-Migration project (which this is a part of) to track these blockers. If you have any more, please do report them in Gerrit-Migration and whatever piece of software it relates to (eg, Differential (arc issues can go in Differential) or Diffusion or whatnot).