Skip to content
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

Remove net5.0 from JaegerExporter #3220

Merged
merged 4 commits into from
Apr 22, 2022

Conversation

cijothomas
Copy link
Member

@cijothomas cijothomas commented Apr 21, 2022

Towards #3147
Its not clear why we have a net5.0 target for Jaeger, as it has netstandard2.1.. @CodeBlanch any thoughts?

@cijothomas cijothomas requested a review from a team April 21, 2022 19:56
@reyang
Copy link
Member

reyang commented Apr 21, 2022

Its not clear why we have a net5.0 target for Jaeger, as it has netstandard2.1.. @CodeBlanch any thoughts?

I guess

#if NET5_0_OR_GREATER
using HttpResponseMessage response = this.httpClient.Send(request);
#else
using HttpResponseMessage response = this.httpClient.SendAsync(request).GetAwaiter().GetResult();
#endif

@cijothomas
Copy link
Member Author

Its not clear why we have a net5.0 target for Jaeger, as it has netstandard2.1.. @CodeBlanch any thoughts?

I guess

#if NET5_0_OR_GREATER
using HttpResponseMessage response = this.httpClient.Send(request);
#else
using HttpResponseMessage response = this.httpClient.SendAsync(request).GetAwaiter().GetResult();
#endif

Thanks! I need to work on my "searching skills!!" :(

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@1918e4b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3220   +/-   ##
=======================================
  Coverage        ?   85.67%           
=======================================
  Files           ?      260           
  Lines           ?     9366           
  Branches        ?        0           
=======================================
  Hits            ?     8024           
  Misses          ?     1342           
  Partials        ?        0           
Impacted Files Coverage Δ
...Exporter.Jaeger/Implementation/JaegerHttpClient.cs 91.66% <ø> (ø)

@cijothomas cijothomas merged commit ad2969d into open-telemetry:main Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants