-
Notifications
You must be signed in to change notification settings - Fork 290
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
[repo/Geneva] Prepare to .NET9 #2342
Conversation
@@ -87,7 +86,6 @@ public static void JsonSerializer_Numeric() | |||
[Trait("Platform", "Any")] | |||
public static void JsonSerializer_String() | |||
{ | |||
TestSerialization((string)null, "null"); |
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.
Why is this line and the two similar ones below being removed?
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.
There is an issue reported, related to redundant casting and there is a separate tests covering such scenario: JsonSerializer_Null
.
BTW most (all) cases are also covered by the generic one TestSerialization
and source in Data
property. Either generic test or specifc tests should be removed.
test/OpenTelemetry.Exporter.Geneva.Tests/OtlpProtobufMetricExporterWithPrefixTests.cs
Outdated
Show resolved
Hide resolved
@@ -87,7 +86,6 @@ public static void JsonSerializer_Numeric() | |||
[Trait("Platform", "Any")] | |||
public static void JsonSerializer_String() | |||
{ | |||
TestSerialization((string)null, "null"); |
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.
There is an issue reported, related to redundant casting and there is a separate tests covering such scenario: JsonSerializer_Null
.
BTW most (all) cases are also covered by the generic one TestSerialization
and source in Data
property. Either generic test or specifc tests should be removed.
test/OpenTelemetry.Exporter.Geneva.Tests/OtlpProtobufMetricExporterWithPrefixTests.cs
Outdated
Show resolved
Hide resolved
test/OpenTelemetry.Exporter.Geneva.Tests/OtlpProtobufMetricExporterWithoutPrefixTests.cs
Outdated
Show resolved
Hide resolved
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
Changes
Follow up #2251
Merge requirement checklist
[ ] Unit tests added/updated[ ] AppropriateCHANGELOG.md
files updated for non-trivial changes[ ] Changes in public API reviewed (if applicable)