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.

Sign up for GitHub

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

Jump to bottom

Renaming TraceContextFormat/B3Format/BaggageFormat #1175

Merged
merged 5 commits into from
Aug 28, 2020
Merged

Renaming TraceContextFormat/B3Format/BaggageFormat #1175

merged 5 commits into from
Aug 28, 2020

Conversation

eddynaka
Copy link
Contributor

@eddynaka eddynaka commented Aug 26, 2020 •
edited
Loading

Refs #1143

Changes

  • Renaming TraceContextFormat to TextMapPropagator
  • B3Format to B3Propagator
  • BaggageFormat to BaggagePropagator

Please provide a brief description of the changes here.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@eddynaka eddynaka requested a review from a team August 26, 2020 17:49
@eddynaka
Copy link
Contributor Author

Today we have 4 classes that implements ITextFormat: B3Format, BaggageFormat, CompositePropagator, TextMapPropagator.

Should we rename B3Format and BaggageFormat to B3Propagator and BaggagePropagator?

@codecov
Copy link

codecov bot commented Aug 26, 2020 •
edited
Loading

Codecov Report

Merging #1175 into master will increase coverage by 0.27%.
The diff coverage is 84.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1175      +/-   ##
==========================================
+ Coverage   77.41%   77.68%   +0.27%     
==========================================
  Files         219      219              
  Lines        6270     6279       +9     
==========================================
+ Hits         4854     4878      +24     
+ Misses       1416     1401      -15
Impacted Files Coverage Δ
src/OpenTelemetry.Api/Metrics/MeasureMetric.cs 25.00% <0.00%> (ø)
...penTelemetry.Api/Metrics/NoopBoundCounterMetric.cs 0.00% <ø> (ø)
...penTelemetry.Api/Metrics/NoopBoundMeasureMetric.cs 0.00% <ø> (ø)
src/OpenTelemetry.Api/Metrics/NoopCounterMetric.cs 14.28% <ø> (ø)
src/OpenTelemetry.Api/Trace/TelemetrySpan.cs 88.23% <ø> (+4.45%) ⬆️
...enTelemetry/Metrics/DoubleBoundCounterMetricSdk.cs 75.00% <ø> (ø)
...enTelemetry/Metrics/DoubleBoundMeasureMetricSdk.cs 0.00% <ø> (ø)
...rc/OpenTelemetry/Metrics/DoubleCounterMetricSdk.cs 45.45% <ø> (ø)
...penTelemetry/Metrics/Int64BoundCounterMetricSdk.cs 75.00% <ø> (ø)
...penTelemetry/Metrics/Int64BoundMeasureMetricSdk.cs 66.66% <ø> (ø)
... and 18 more

@CodeBlanch
Copy link
Member

Should we rename B3Format and BaggageFormat to B3Propagator and BaggagePropagator?

I like it!

@eddynaka
Renaming TraceContextFormat to TextMapPropagator
4f58c3b 
updating changelog

renaming b3Format to B3PRopagator

renaming BaggageFormat to BaggagePropagator

renaming file
@eddynaka eddynaka changed the title Renaming TraceContextFormat to TextMapPropagator Renaming TraceContextFormat/B3Format/BaggageFormat Aug 27, 2020
@CodeBlanch
Copy link
Member

Not on this PR, but maybe we should rename ITextFormat -> IPropagator?

@eddynaka
Copy link
Contributor Author

Not on this PR, but maybe we should rename ITextFormat -> IPropagator?

i would like that as well! lets finish this and do that next! :)

@cijothomas cijothomas merged commit da72b13 into open-telemetry:master Aug 28, 2020
@eddynaka eddynaka deleted the feature/renaming-tracecontextformat branch August 28, 2020 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cijothomas cijothomas cijothomas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eddynaka @CodeBlanch @cijothomas