[SPIKE] Don't do "clean" check before attempting to check-out PR

[jcansdale]

NOTE: This PR is intended for experimenting with functionality rather than necessarily merging.

In previous versions, the PR Checkout link was only active if the working directory and latest commit where exactly the same. There isn't this restriction when checking out using command line Git.

In #1302 this restriction is relaxed a bit to allow untracked files in the staging area (often build artifacts or log files). This PR changes it to behave more like the command line. It will always attempt a checkout and inform the user if it fails.

• For example, if the user makes a local change.

The Checkout command will work and bring that change to the PR branch. There currently isn't any indication that there are local changes when using the PR pane.

• If there are local changes that conflict with the PR being checked out, an error message will be displayed.

It doesn't currently give any hints how to fix the conflicts. Maybe we should offer the user a link to the Team Explorer - Changes pane?

Fixes #1271.

[grokys]

It doesn't currently give any hints how to fix the conflicts. Maybe we should offer the user a link to the Team Explorer - Changes pane?

Yes, I think that would be the best solution.

[jcansdale]

Here are some related issues:

• Can't checkout PR when there are untracked files: Can't checkout PR when there are untracked files #1271
• Submodules cause all kinds of issues when checking out a PR: Submodules cause all kinds of issues when checking out a PR #826
• Explicitly display "can't checkout" message: Explicitly display "can't checkout" message  #998

Initially thought to be related, but maybe this PR is a simpler solution:

• What git commands should we execute when checking out a PR What git commands should we execute when checking out a PR #848

Also related:

• Feature: Maintainer workflow Feature: Maintainer workflow #491

[meaghanlewis]

Hey @jcansdale thanks for making these changes! I was testing this and have a few questions.

• Where does the message 1 conflict prevents checkout come from? I can't find it in the codebase anywhere.
• I get the conflict message sometimes even when I've changed files that aren't related to a PR. For example, I changed a file (src\GitHub.UI\GitHub.UI.csproj) that wasn't included in the files changed in this PR and then tried to check out this branch fixes/1271-just-try-PR-checkout. This is unexpected, right?
• Is clicking [GitChanges] supposed to take me to Team Explorer - Changes? Currently it's not doing anything for me.

• Unrelated to functionality, but I noticed that tests were removed for this PR but no new ones were added. Should they be added? I'd be up to pair to add a test or two (especially because I currently have no idea how to write tests for this project)!

[jcansdale]

Where does the message 1 conflict prevents checkout come from? I can't find it in the codebase anywhere.

This is a the message from a libgit2sharp exception that gets thrown when it fails to checkout (kind of like when command line Git can't do a checkout).

I get the conflict message sometimes even when I've changed files that aren't related to a PR. For example, I changed a file (src\GitHub.UI\GitHub.UI.csproj) that wasn't included in the files changed in this PR and then tried to check out this branch fixes/1271-just-try-PR-checkout. This is unexpected, right?

I think changing files that are included in the PR might be okay, it's changing files that are changed on the target branch that is an issue.

Is clicking [GitChanges] supposed to take me to Team Explorer - Changes? Currently it's not doing anything for me.

Yup, that is how it's supposed to work! I'll check.

Unrelated to functionality, but I noticed that tests were removed for this PR but no new ones were added. Should they be added?

In this case the tests were for a method that has been removed. To test the new functionality, we would need to do integration tests against a real Git repo. This might be a little involved, so I think it would make sense to flesh out exactly how we want it to work before tackling this. It definitely makes sense to add them though.

[jcansdale]

This PR was an experiment at addressing #1271. This has since been fixed by the following PRs:

• Don't block checking out a PR if there are untracked files Don't block checking out a PR if there are untracked files #1302
• [Feature] Reviewing PR with submodule changes [Feature] Reviewing PR with submodule changes #1392