Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set resources correctly on CopyRefAssembly #55893

Merged
merged 1 commit into from
Aug 26, 2021
Merged

Conversation

jaredpar
Copy link
Member

Similar to other tasks we fixed previously we need to ensure that
resources are set correctly when creating the CopyRefAssembly task.
The only correct way to do this is pass the ResourceManager through
the constructor.

Follow up of #52836
Related to dotnet/msbuild#6253

Similar to other tasks we fixed previously we need to ensure that
resources are set correctly when creating the `CopyRefAssembly` task.
The only correct way to do this is pass the `ResourceManager` through
the constructor.

Follow up of dotnet#52836
Related to dotnet/msbuild#6253
@jaredpar jaredpar requested a review from a team as a code owner August 25, 2021 19:25
@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler PTAL

@RikkiGibson RikkiGibson self-assigned this Aug 25, 2021
using Microsoft.Build.Framework;
using Microsoft.CodeAnalysis.BuildTasks;
using Xunit;
using Moq;
Copy link
Member

Choose a reason for hiding this comment

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

Are we using this?

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 1)

@jcouv jcouv self-assigned this Aug 26, 2021
@jaredpar jaredpar merged commit 1dc78ad into dotnet:main Aug 26, 2021
@ghost ghost added this to the Next milestone Aug 26, 2021
@jaredpar jaredpar deleted the error branch August 26, 2021 03:43
@dibarbet dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants