Page MenuHomePhabricator

arc commandeer
Closed, ResolvedPublic

Description

I can't see any way to commandeer through arc. I feel like this would be a nice convenience method around conduit's differential.revision.edit.

Event Timeline

Willyham created this task.Feb 10 2017, 4:20 PM

Other than implementing arc commandeer, can you suggest two different ways we could solve the problem you're running into?

(See: Contributing Feature Requests and Describing Root Problems for more.)

I'm not trying to be facetious, but I'm not sure that there are two other ways. The scenario is that somebody is out and I'd like to continuing working on their feature whilst they're away. I've read through T10584 which mostly discusses why commandeer should be used in this case rather than updating somebody else's diff. In this case, if they say to me, "hey, can you take D123 whilst I'm out?", I'd prefer to just type arc commandeer D123 than have to go to the UI, find the diff and commandeer it. I suppose that I'm being lazy, but the exact same argument exists for arc close-revision or arc upload.

Now, I have another use case for this too. The root problem here is that I have a diff which I'd like somebody else to update. I currently find myself in the situation where:

  • I've created a diff
  • The diff doesn't build because I need to run a command, let's call it make foo
  • make foo doesn't work on my machine, because I'm using a 'non-standard' setup temporarily.

This isn't something which happens often, but more of a one-off scenario. I want to make it as simple for somebody else to run make foo for me and update the diff. What I'd like to be able to do is comment on the diff and ask another team member to copy/paste a command to do this in one shot:

Hey, @foo, can you please run git checkout master && arc commandeer D123 && arc patch D123 && git checkout arcpatch-D123 && make foo && arc diff

I suppose that this use case is trying to subvert the fact that only the author can edit the diff, but I literally need somebody else to update the diff or it cannot move forwards. If commandeering is the way to solve that problem, then I would like commandeering to be simple.

chad added a subscriber: chad.Feb 10 2017, 5:16 PM

Can't they just arc patch Dxxxx and arc diff their changes back up?

@chad my understanding was that only the current owner of a revision can make changes. I guess I didn't read that thread through to completion, though I'm not sure that patch has made it to our install yet.

I suppose that I'm being lazy, but the exact same argument exists for arc close-revision or arc upload.

arc close-revision exists mostly for historical reasons. We probably would not add it today.

arc upload exists so you can upload files from a remote system (or a container, or whatever) which doesn't have a browser/GUI. I think this one is actually pretty useful if you do CLI stuff / remote development.

arc commandeer feels like a way to very rarely save about two seconds. You can already arc browse D123 if finding the diff or typing D123 or /D123 into various places is onerous, so it only saves scrolling to the bottom of the page (or pressing "Z") and two inputs. But the cost is that arc help gets bigger and arc gets more overwhelming for new users, and we have to maintain a little bit more stuff. If we added arc commandeer, I think there would be a reasonable argument that it might be the least useful arc command.

You can also already do this by writing a script/alias that boils down to this:

echo '{
  "objectIdentifier": "'$1'",
  "transactions": [
    {
      "type": "commandeer"
    ]
  ]
}' | arc call-conduit differential.revision.edit

T7787 may provide another simpler way to build this command from more powerful building blocks.

I suppose that this use case is trying to subvert the fact that only the author can edit the diff

D15468 came out of T10584, and allows this workflow to work. User @foo will receive a warning ("Really update a revision you don't own?") but will be allowed to proceed. So the Hey @foo, ... workflow should already work as long as arc is newer than ~March 2016.

chad added a comment.Feb 10 2017, 5:28 PM

This is what I just did as a test:

chad@phac-dev:/var/www/html/dev/phabricator$ arc patch D17269
chad@phac-dev:/var/www/html/dev/phabricator$ touch TEST
chad@phac-dev:/var/www/html/dev/phabricator$ git add -A
chad@phac-dev:/var/www/html/dev/phabricator$ git commit
[arcpatch-D17269 45664cf] test file
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 TEST
chad@phac-dev:/var/www/html/dev/phabricator$ arc diff


    You don't own revision D17269: "Add a "Red/Green Colorblind"
    accessibility mode, make all web UIs and email respect it". Normally, you
    should only update revisions you own. You can "Commandeer" this revision
    from the web interface if you want to become the owner.
    
    Update this revision anyway? [y/N]

Yep, if you can update other people's diffs then this is fine. I'll just upgrade if needed. Thanks.

Ironically, I don't seem be able to close my own task.

epriestley closed this task as Resolved.Feb 10 2017, 5:48 PM
epriestley claimed this task.
chad added a comment.Feb 10 2017, 5:50 PM

๐Ÿ’ƒ