-
Notifications
You must be signed in to change notification settings - Fork 773
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
Metrics to Main #2174
Metrics to Main #2174
Conversation
…expected to come from the API callbacks (#2134)
Update DiagnosticSource version
Codecov Report
@@ Coverage Diff @@
## main #2174 +/- ##
==========================================
- Coverage 82.12% 74.57% -7.56%
==========================================
Files 250 218 -32
Lines 6774 6957 +183
==========================================
- Hits 5563 5188 -375
- Misses 1211 1769 +558
|
<PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="$(MicrosoftCodeAnalysisAnalyzersPkgVer)" Condition=" $(OS) == 'Windows_NT'"> | ||
<PrivateAssets>All</PrivateAssets> | ||
</PackageReference> |
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.
We will continue to leave this disabled on main for now?
* Removed existing Metrics code as the spec is completely being re-written. | ||
([#2030](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2030)) | ||
|
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.
Might make sense to update this changelog to reflect the new state of things.
* Removed existing Metrics code as the spec is completely being re-written. | ||
([#2030](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2030)) |
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.
Same thing for the SDK changelog
@@ -34,7 +34,7 @@ public TracerProviderSdkTest() | |||
Activity.DefaultIdFormat = ActivityIdFormat.W3C; | |||
} | |||
|
|||
[Fact] | |||
[Fact(Skip = "Get around GitHub failure")] |
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.
Is it still necessary to skip this test on the main branch?
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.
LGTM
@@ -81,9 +84,6 @@ Released 2021-Jan-29 | |||
* `Status.WithDescription` will now ignore the provided description if the | |||
`Status.StatusCode` is anything other than `ERROR`. | |||
([#1655](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1655)) | |||
* Metrics removed as it is not part 1.0.0 release. See issue |
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 guess we need to keep this.
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.
LGTM. After this PR got merged, might need to update the CI definition by removing metrics branch.
We are prepping for 1.2.0-alpha1 release. 1.2.0 is the next planned release, and now we can work in main branch for metrics code as well.