I think the article is assuming the base for `arc diff` is always the parent revision, i.e. `arc diff HEAD^`, which is how the workflow works best. Strangely I don't think the open source Phabricator is set up to do this by default so you have to actually type `arc diff HEAD^` (there's probably some setting somewhere so that you can make this the default). On the diff in Phabricator you can enter the dependencies manually. Really the tooling ought to do this for you (and at Facebook our internal tooling does do this) but for now manually specifying the dependencies is not terrible. Then Phabricator shows you the nice dependency tree in the UI, so you can see the state of all of your diffs in the stack. Cheers Simon On Fri, 5 Oct 2018 at 04:30, Niklas Hambüchen wrote:

There are some things in these argumentations that I don't get.

When you have a stack of commits on top of master, like:

* C | * B | * A | * master

What do you use as base for `arc diff` for each of them?

If B depends on A (the patch expressed by B doesn't apply if A was applied first), do you still use master as a base for B, or do you use Phabricator's feature to have diffs depend on other diffs?

On Fri, 5 Oct 2018 at 15:22, Niklas Hambüchen wrote:

...

I think the article is assuming the base for `arc diff` is always the parent revision, i.e. `arc diff HEAD^`, which is how the workflow works best. Strangely I don't think the open source Phabricator is set up to do this by default so you have to actually type `arc diff HEAD^`

Perhaps that is exactly to address the problem in my example: If you submit a patch B that depends on A, by default this patch will fail to apply against master on the Phabricator side unless you manually set up dependencies? I suppose this is why it defaults to submitting the whole master-A-B history instead?

...

for now manually specifying the dependencies is not terrible.

I have found it pretty terrible: Setting up dependencies between commits by hand is time consuming, and you can do it wrong, which easily leads to confusion.

If I do 4 refactor commits and on top a new feature that needs them, why should I have to manually click together the dependencies between those commits? The whole point of git is that it tracks that already for me in its DAG.

It gets worse if I have to react to review feedback:

Say Ben tells me in review that I should really squash commits 2 and 3 because they don't work independent of each other. Easily done with `git rebase -i` as suggested, but now I have to go and reflect what I just did in version control by manual clicking in an external tool again (and I better kick out the right Diff).

Similarly, if want to rename all occurrences of my_var to myVar across my 5 commits using rebase -i, I have to manually invoke the right arc invocation after each commit.

So I've found it a big pain to maintain a series of dependent commits with this workflow.

I can imagine this to be only painless if you have access to the tooling you said you have at facebook, that automates these things for you.

In fact we did it manually for a long time, the tool support is a recent development. Tool support can always improve things, but I'll take the inconvenience of having to specify dependencies manually in exchange for the other benefits of stacked diffs. You can put the dependencies in the commit log using "Depends on: D1234", as an alternative to the UI. 'git rebase -i' with 'x arc diff HEAD^ -m rebase' is a nice trick for rebasing your stack. Cheers Simon

In my ideal world, it should work like this:

* Locally, a series of dependent patches goes into a git branch. * Branches that are dependent on each other are based on each other. * You have a tool that, if you amend a commit in a branch, can rebase all the dependent branches accordingly. * You can tell `arc` to submit a whole branch, and it will automatically upload all dependent branches and set up the Phabricator dependency relationships for you. * When you react to review feedback, you change your history locally, and run an `arc upload-changes`, that automatically updates all Diffs accordingly.

Niklas