Entry point

A contributor opens a PR

Reviewing a PR

In the PR list, click on "Review PR" to open the PR Review (details) pane

• Adds PR remote to your local clone, if not there yet
• If it's the first time this PR is being reviewed through VS and we've just added the remote
  • Create local branch tracking remote PR branch
    • If local branch matching naming convention already exists and is tracking the correct remote PR branch, assume that as the local PR branch
  • If local branch matching naming convention already exists but is not tracking the correct remote branch
    • Create local branch with a numbered suffix
• Find local branch matching naming convention of PR with correct remote tracking branch of PR (should probably cache this info)
• Switch to local branch

PR Review UI:

• Tree view of changed files
  • Open individual file diff from tree view
• Link to conversation view in PR Review UI
  • so you can jump to github to comment on the PR
• If there are new commits since the last time this PR was opened:
  • Shows message saying "New commits available"
  • If there is a local branch for the PR, message says "would you like to update your local branch?"
    • If yes, fast-forwards the local branch.
      • If there are local changes then we warn and don't do anything

Merging and Closing a PR

The PR Review/Details pane has a Merge and a Close button.

• Click on "Merge PR" or "Close PR"
  • Perform the action via the API
  • In case of merge:
    • Ask if user wants to clean up the PR branch
      • If yes, delete the remote and local branches
      • Fast forward target branch with remote commits
      • Switch to target branch

Asks the user if they want to create a branch to test the PR or select an existing one

What is the use-case for this? I'd assumed that just checking out the PR into a new branch would suffice. At least that's the way that desktop does it.

Here are my notes from the initial spike I did last week. It goes into a little more detail on how things might work in the PR list pane.

Flow for Maintainer Workflow

• In PR list, each open PR has a "checkout" button if no local branch
• If there is a local branch for a PR it also has "merge" and "sync" icon buttons
• Clicking "checkout" checks out PR as local branch and switches to that branch
  • If already checked out into a branch, switches to that branch
• "Sync" refreshes from upstream PR
• "Merge" merges the PR to master and switches to master
• Once merged, a "Delete" button appears - this deletes the local and remote branch (if PR from same repos)

Questions

• How do we name local PR branches? Like in Desktop ("pr/123"), or could we name it more intelligent (e.g. "pr/123-this-does-that)?
• Would a user making changes on a PR branch cause problems? e.g. with subsequent sync?
• Can we always tell if a branch is the PR branch we're looking for? What if the user has already checked out the PR in desktop or with command-line? Do we reuse those?
• Should "delete" button always be present on local PR to delete local branch? Would the difference in behavior be confusing? i.e. that sometimes it just deletes local and sometimes both local and remote.

What is the use-case for this? I'd assumed that just checking out the PR into a new branch would suffice. At least that's the way that desktop does it.

If there's already an existing branch that matches the naming pattern that we use, then it's likely the user has already created a branch for the PR and somehow we lost track of it. We need to decide whether to use a different name for the branch or ask the user if they want to use the existing one, or disable the feature entirely and let the user switch branches. But this would only happen if the repo is already set up to do PRs before, so this is on the wrong part of the flow.

Once synced, a "Delete" button appears - this deletes the local and remote branch (if PR from same repos)

Do you mean "Once merged" here?

If there is a local branch for a PR it also has "merge" and "sync" icon buttons

Sync is way too overloaded with meaning to use here, people will assume we're pushing things. "Update" or "Update local branch" or something more clear is probably preferable.

How do we name local PR branches? Like in Desktop ("pr/123"), or could we name it more intelligent (e.g. "pr/123-this-does-that)?

"pr/###-title-of-PR" would be nicer than just the number.

Would a user making changes on a PR branch cause problems? e.g. with subsequent sync?

If they make changes and don't commit, we can't do anything to the local branch. If they make changes and commit them, we need to decide whether we want to do a fetch&merge or tell them to do it themselves via the git UI.

Can we always tell if a branch is the PR branch we're looking for? What if the user has already checked out the PR in desktop or with command-line? Do we reuse those?

We can always tell if a local branch is appropriate by checking if it's tracking a remote branch and whether it matches the remote branch with no local changes. This is where we should probably ask the user if they want to reuse an existing branch.

Should "delete" button always be present on local PR to delete local branch? Would the difference in behavior be confusing? i.e. that sometimes it just deletes local and sometimes both local and remote.

The local one is always deletable once the PR is merged. Dunno what to do about the remote one, I agree that it's going to be confusing if "Clean up" means different things depending on whether you have permissions to the repo or not.

Separate feature idea: "Remote clean up" button that clears all remote branches whose tips are already on master, and "Local clean up" button that prunes all useless local branches whose tips are already on master.

Ugh, I could really use a list of files that go with this PR right so I can quickly jump to them.