Create Task
Maniphest T5946

updating a raw diff through arcanist resets the repository info
Open, Needs TriagePublic
Actions

Description

Something I've noticed in the past but which seems to have become systematic with the latest arcanist version:

  1. create a review request ("differential revision") with all fields filled in, including the repository the diff applies to.
  2. use arcanist to upload an updated raw diff, e.g. arc diff --update DXXXX --raw < foo.diff --message "raw diff updated with arc" .
  3. use "Edit Revision" to check the repository field (why is this info not shown?) .

When I follow these steps, the repository information has been lost.

I notice that it apparently has to be entered anew when updating a diff through the web interface.

In itself it is fine that it's possible to update a diff and the repository information, but IMHO it shouldn't be required (or else arc diff should provide an argument to provide this data along with the new diff).

rjvbb created this task.Apr 24 2017, 7:14 PM
bcooksley added a subscriber: bcooksley.Apr 24 2017, 7:16 PM

I'll ask upstream but this looks like fairly expected behaviour to me. It's each diff that belongs to a repository not the revision itself.

rjvbb added a comment.Apr 24 2017, 8:40 PM

But why would the other fields remain set? If you change the repo you could also argue that the subscribers and reviewers might have to change.

As I said, it's good that you can change the repo, but I don't expect this to be the default for a diff *update* in a revision process. You do that (updating) when changes have been requested, why would that change anything to the target repo except in probably rare exceptions (like when a repository is split)?

Uncoupling the revision (aka review request) from the diff seems far-fetched to me; each revision concerns a diff which in turn applies to a repository.

bcooksley added a comment.Apr 24 2017, 8:41 PM

Subscribers and Reviewers belong to the Revision, not the Diff. They're reviewing the current status of a Revision, which is represented by a Diff, along with various items like a Summary, Test Plan, etc.

rjvbb added a comment.EditedApr 24 2017, 8:58 PM

Well, to me that's making things needlessly complicated, for typical FOSS usage at least. Even more so because the arc command to modify revisions is called diff ...

If updating a diff cannot be done otherwise than combined with a confirmation that it still applies to the same repository than I'd argue that

  • arcanist should provide a way to know which repo a diff/revision currently applies to (= the repo's ID on phabricator). For instance as an optional output field for arc list.
  • arc diff should have a --repository ID option