-
Notifications
You must be signed in to change notification settings - Fork 784
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
Adding conditional check for PublicApiAnalyzers. #3497
Adding conditional check for PublicApiAnalyzers. #3497
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3497 +/- ##
==========================================
+ Coverage 86.28% 86.34% +0.06%
==========================================
Files 270 270
Lines 9865 9865
==========================================
+ Hits 8512 8518 +6
+ Misses 1353 1347 -6
|
build/Common.prod.props
Outdated
@@ -1,7 +1,7 @@ | |||
<Project> | |||
<Import Project=".\Common.props" /> | |||
|
|||
<ItemGroup> | |||
<ItemGroup Condition="'$(ForRelease)' == 'true'"> |
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.
where is this defined?
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.
It will be referenced in the Prometheus shared project.
See #3430 (comment)
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.
Maybe we can shove the shared files under the Prometheus HttpListener project, and have the ASP.NET Core binding stealing from it?
It looks like we have a problem that is not generic, and here we try to solve it by abstraction and generalization (which doesn't seem to pay off IMHO) 😸
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.
would Condition="$(MSBuildProjectName.EndsWith('.Shared'))"
work?
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.
@TimothyMothra, it works!
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.
Seems to be a very uncommon case.
Changes
Not all projects under src/ are meant for releasing.
The
OpenTelemetry.Exporter.Prometheus.Shared
project in this PR https://github.com/open-telemetry/opentelemetry-dotnet/pull/3430/files is one example.PublicApi documents are not required for projects not for release.
Thus, adding conditional check for PublicApiAnalyzers for the projects not meant for releasing to utilize it.