Page MenuHomePhabricator

Let "Land Revision" target branches other than master
Closed, ResolvedPublic

Assigned To
Authored By
epriestley
Dec 10 2015, 8:45 PM
Referenced Files
F1025927: pasted_file
Dec 14 2015, 10:22 PM
F1020636: Screen Shot 2015-12-10 at 3.03.02 PM.png
Dec 10 2015, 11:38 PM
Tokens
"Like" token, awarded by joshuaspence."Yellow Medal" token, awarded by avivey.

Description

Currently, "Land Revision" always lands into master. Here's my plan to fix this, so it can be used to land into other branches:

  • Add PHIDs to PhabricatorRepositoryRefCursor. This will allow us to create a "Branch: ..." typeahead in a clean way.
  • Create a typeahead datasource for repository refs (like branches).
  • Add a "Branch:" field to the "Land" dialog, using this new datasource and defaulting to the "default branch" for the repository as specified in Diffusion. This will let the user choose which branch they want to target, although the default option sometimes won't be right.
  • Make sure the repository operation UI surfaces this information.
  • Implement T3462 in arc for tracking branches, so we start storing "branched from <closest upstream branch>" on diffs and showing it in the UI. There is some complexity / future work here that I'll discuss in T3462.
  • When this information is available on a diff, use it to select a better default branch than the Diffusion default. This will mean that we get the branch the user wants right more often.

Event Timeline

epriestley claimed this task.
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added a project: Differential.
epriestley added subscribers: epriestley, nickz, chad.

This behavior should be more reasonable at HEAD. In particular:

  • "Land Revision" now lets you choose a branch to land onto.
  • Most of the time, it should choose a reasonable default.
  • If it doesn't, you can pick the branch you really want.

T3462 has more details about guessing where to land. Broadly, we choose a default by looking at:

  • if feature tracks an upstream branch (or tracks a local branch which tracks an upstream branch), we'll use that;
    • (you can use arc feature or git checkout -t or similar to set up branch tracking)
  • otherwise, if arc.land.onto.default was set when the diff was created, we'll use that;
  • otherwise, we'll fall back to the "Default Branch" configured for the repository in Diffusion (usually "master", but you can set this to something else).

The new tracking features require an up-to-date version of arc to work. When they're working, you should see "feature (branched from upstream)" in the "Branch" field in the UI, like this:

Screen Shot 2015-12-10 at 3.03.02 PM.png (171×420 px, 28 KB)

If you see that, it means that "Land Revision" will default to upstream (in this screenshot, "test").

I tested this feature and noticed that the browse button beside "Onto Branch" was not clickable unless I removed the pre-populated default branch.

pasted_file (334×598 px, 46 KB)

Ideally, the user should be able to see the available branches after clicking the browse button no matter whether a branch is already in that field.

Ah, yeah, that's reasonable.

I disabled the button on all only-one-item tokenizers once they have an item in D14738 because users found it confusing on fields like "Owner:" in Maniphest (they thought they could assign several owners to a task, e.g. see T9955) but I think the behavior you propose (allow browse, but replace the value) is better than the one I chose. I'll change it.

Thanks for taking care of this!