Skip to content

Single automatic assembly pack package reference#11242

Merged
dsplaisted merged 3 commits into
dotnet:masterfrom
sfoslund:RefAssemblyPackReference
Apr 17, 2020
Merged

Single automatic assembly pack package reference#11242
dsplaisted merged 3 commits into
dotnet:masterfrom
sfoslund:RefAssemblyPackReference

Conversation

@sfoslund
Copy link
Copy Markdown
Member

  • Changing BeforeTargets so package reference is only added once in a multitargeted project, instead of once per .NET framework TFM.
  • Adding test coverage to ensure no warnings generated

Should fix warnings we're seeing with #11188

@sfoslund sfoslund requested a review from dsplaisted April 11, 2020 00:12
Copy link
Copy Markdown
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

It looks like this doesn't actually prevent the duplicate references, it just inserts the duplicate reference after the check that's supposed to catch them.

@sfoslund sfoslund force-pushed the RefAssemblyPackReference branch from 207c56d to 2c3b3f6 Compare April 14, 2020 00:54
@sfoslund
Copy link
Copy Markdown
Member Author

@dsplaisted my bad, this should be fixed in the new changes, thanks!

@sfoslund
Copy link
Copy Markdown
Member Author

@dsplaisted this is blocking a dependency PR into master, do you mind giving it another look now that I've pushed updates? Thanks!

Copy link
Copy Markdown
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Can you add a test that verifies that everything works and there are no warnings if you have an explicit reference to the package?

@sfoslund
Copy link
Copy Markdown
Member Author

@dsplaisted this test should verify that already, I added a check for the warning so it fails without this fix now

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.

2 participants