-
Notifications
You must be signed in to change notification settings - Fork 772
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
Extract TFM for DLLs shipped from this repo #4903
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,12 @@ | |
|
||
<PropertyGroup> | ||
<!-- OmniSharp/VS Code requires TargetFrameworks to be in descending order for IntelliSense and analysis. --> | ||
<DefaultTargetFrameworks>net6.0;netstandard2.0;net462</DefaultTargetFrameworks> | ||
<TargetFrameworksForLibraries>net6.0;netstandard2.0;net462</TargetFrameworksForLibraries> | ||
<TargetFrameworksForLibrariesExtended>net6.0;netstandard2.1;netstandard2.0;net462</TargetFrameworksForLibrariesExtended> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious - what's the rationale behind this naming? (I saw it's used for OTLP exporter and OpenTelemetry project.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a name for libs that need to target extra netstandard TFM.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possible to do this here...
...or order needs to be preserved? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel inheritance is not adding much value, and is creating issue for IDEs/tools. |
||
<TargetFrameworksForAspNetCoreInstrumentation>net7.0;net6.0;netstandard2.1;netstandard2.0</TargetFrameworksForAspNetCoreInstrumentation> | ||
<TargetFrameworksForGrpcNetClientInstrumentation>net6.0;netstandard2.1;netstandard2.0</TargetFrameworksForGrpcNetClientInstrumentation> | ||
<TargetFrameworksForPrometheusAspNetCore>net6.0</TargetFrameworksForPrometheusAspNetCore> | ||
|
||
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)/OpenTelemetry.prod.ruleset</CodeAnalysisRuleSet> | ||
<RunApiCompat>true</RunApiCompat> | ||
<ApiCompatExcludeAttributeList>$(RepoRoot)\build\GlobalAttrExclusions.txt</ApiCompatExcludeAttributeList> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since everything in prod.props is a Library anyway, we could use diff. name by avoiding "library"...
DefaultTargetFrameworks, DefaultTargetFrameworksExtended seems okay to me plus the
TargetFrameworksForAspNetCoreInstrumentation, etc..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel the word "default" is generating confusion. The way I look at it is that:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at what I was doing in #4902, I consider this as "default" (the individual projects don't have to add TargetFrameworks at all since the props file covered it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @cijothomas I don't love "Libraries" in the name because all of these things are really libraries in some regard but I'm fine to leave as-is and we can continue to mess with it 😄