Drop the net35 asset from the StringTools package #8198
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As discussed offline, the StringTools package is currently not authored correctly as the StringTools assembly is differently named between target frameworks. More precisely, the net35 assembly is named differently than the rest of the builds: Microsoft.NET.StringTools.net35.dll. Reason for that is that this assembly is binplaced into a common folder with another tfm build of the same library. By dropping the net35 asset from the package, assembly FileNotFoundExceptions are avoided.
ladipro
suggested changes
Nov 28, 2022
Member
ladipro
left a comment
ladipro
left a comment
There was a problem hiding this comment.
It looks like net35 should be excluded from some builds after all, see the CI failure.
Member
Author
|
Thanks, addressed the feedback. Double checked locally. The package now contains all the expected assets. |
Forgind
approved these changes
Nov 29, 2022
src/StringTools/StringTools.csproj
Outdated
| <!-- Don't publish the reference assembly if the build output isn't included. --> | ||
| <TargetsForTfmSpecificBuildOutput Condition="'$(IncludeBuildOutput)' == 'false'" /> | ||
| <!-- NU5128: Add lib or ref assemblies for the net35 target framework. --> | ||
| <NoWarn>$(NoWarn),NU5128</NoWarn> |
Contributor
There was a problem hiding this comment.
I think we normally use semicolons. I imagine it's ok with mixed commas and semicolons, but it's good to be consistent.
Member
Author
There was a problem hiding this comment.
Thanks, updated to use a semicolon as the delimiter here. The official documentation states that you would use commas only for vbproj projects.
Forgind
added
the
merge-when-branch-open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed offline, the StringTools package is currently not authored correctly as the StringTools assembly is differently named between target frameworks. More precisely, the net35 assembly is named differently than the rest of the builds: Microsoft.NET.StringTools.net35.dll. Reason for that is that this assembly is binplaced into a common folder with another tfm build of the same library.
By dropping the net35 asset from the package, assembly FileNotFoundExceptions are avoided.
Contributes to #8116