Skip to content

Fix API breaking change from adding SdkResolver IndicateSuccess overload#5435

Merged
dsplaisted merged 1 commit intodotnet:masterfrom
dsplaisted:fix-resolver-api-break
Jun 17, 2020
Merged

Fix API breaking change from adding SdkResolver IndicateSuccess overload#5435
dsplaisted merged 1 commit intodotnet:masterfrom
dsplaisted:fix-resolver-api-break

Conversation

@dsplaisted
Copy link
Copy Markdown
Member

#5269 introduced an API breaking change by adding an overload to the IndicateSuccess method of the SdkResultFactory abstract class.

This class should only be implemented by MSBuild itself and by implementors of SDK resolvers when they want to mock this class for testing. So the impact of the break is probably low, but it still seems like a good idea to fix.

@dsplaisted dsplaisted requested a review from rainersigwald June 16, 2020 19:48
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.

LGTM

@dsplaisted
Copy link
Copy Markdown
Member Author

@Forgind @rainersigwald Any idea why there are test failures for ItemTransformContainingSemicolon_InTaskHost in this PR?

@Forgind
Copy link
Copy Markdown
Contributor

Forgind commented Jun 16, 2020

#5423
The test started failing a lot recently, and we don't know why, although we have a few ideas. Sadly, it often seems to start working when we add logging. I'd be happy to show you what I have so far if you're interested.

@dsplaisted
Copy link
Copy Markdown
Member Author

@Forgind So do you typically just re-run it?

@Forgind
Copy link
Copy Markdown
Contributor

Forgind commented Jun 17, 2020

That, wait for me to figure out what's wrong, or help me figure out what's wrong.

@rainersigwald rainersigwald added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jun 17, 2020
@dsplaisted dsplaisted merged commit 6019483 into dotnet:master Jun 17, 2020
@ghost ghost added this to the current-release milestone Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants