Skip to content

Updating setting SDK resolver isRunningInVisualStudio#5562

Merged
Forgind merged 1 commit into
dotnet:masterfrom
sfoslund:ResolverRunningInVS
Jul 31, 2020
Merged

Updating setting SDK resolver isRunningInVisualStudio#5562
Forgind merged 1 commit into
dotnet:masterfrom
sfoslund:ResolverRunningInVS

Conversation

@sfoslund
Copy link
Copy Markdown
Member

Tested manually along with dotnet/sdk#12633

@sfoslund
Copy link
Copy Markdown
Member Author

@Forgind @benvillalobos this change was suggested by Rainer on teams

Copy link
Copy Markdown
Member

@benvillalobos benvillalobos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You tested it and this uses code that's already being used immediately after. LGTM!

Copy link
Copy Markdown
Contributor

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to keep the RunningInVisualStudio version? If not, may as well remove it.

@benvillalobos
Copy link
Copy Markdown
Member

@Forgind this seems low risk and unblocks Sarah. Let's prioritize it in the PR meeting tomorrow.

@sfoslund
Copy link
Copy Markdown
Member Author

Is there any reason to keep the RunningInVisualStudio version? If not, may as well remove it.

@Forgind I'm not sure what you mean, can you clarify? This change doesn't remove anything, it just changes the way this context property is set.

@Forgind
Copy link
Copy Markdown
Contributor

Forgind commented Jul 30, 2020

I was thinking of #5501. You wanted that so you could do the thing you just replaced, right? Was there more?

@sfoslund
Copy link
Copy Markdown
Member Author

#5501 set the groundwork for this change as well as added an overload for the SDK resolver IndicateSuccess method, so none of that can be removed.

@Forgind Forgind merged commit 44dee6e into dotnet:master Jul 31, 2020
@sfoslund sfoslund deleted the ResolverRunningInVS branch July 31, 2020 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants