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

chore: remove unused default argument in Tracer #913

Merged

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Mar 30, 2020

A Tracer is always created by a TracerProvider and this one always passes it's config
therefore no need to default in constructor.

A Tracer is always created by a TracerProvider and this one always passes it's config
therefore no need to default in construtor.
@codecov-io
Copy link

codecov-io commented Mar 30, 2020

Codecov Report

Merging #913 into master will decrease coverage by 1.56%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #913      +/-   ##
==========================================
- Coverage   94.49%   92.92%   -1.57%     
==========================================
  Files         246      246              
  Lines       10898    11159     +261     
  Branches     1044     1091      +47     
==========================================
+ Hits        10298    10370      +72     
- Misses        600      789     +189     
Impacted Files Coverage Δ
packages/opentelemetry-tracing/src/Tracer.ts 100.00% <100.00%> (+1.78%) ⬆️
...entelemetry-core/test/context/B3Propagator.test.ts 45.53% <0.00%> (-54.47%) ⬇️
packages/opentelemetry-core/test/utils/url.test.ts 50.00% <0.00%> (-50.00%) ⬇️
...pentelemetry-core/test/internal/validators.test.ts 50.00% <0.00%> (-50.00%) ⬇️
...elemetry-core/test/trace/spancontext-utils.test.ts 55.55% <0.00%> (-44.45%) ⬇️
...lemetry-core/test/trace/ProbabilitySampler.test.ts 61.70% <0.00%> (-38.30%) ⬇️
...s/opentelemetry-core/test/trace/tracestate.test.ts 65.06% <0.00%> (-34.94%) ⬇️
...ckages/opentelemetry-core/test/platform/id.test.ts 66.66% <0.00%> (-33.34%) ⬇️
...ntelemetry-core/test/trace/NoRecordingSpan.test.ts 71.42% <0.00%> (-28.58%) ⬇️
...ages/opentelemetry-plugin-http/test/utils/utils.ts 33.33% <0.00%> (-26.67%) ⬇️
... and 51 more

@dyladan
Copy link
Member

dyladan commented Mar 30, 2020

Tracers should not be created manually anyway so I think this is a good change.

Copy link
Member

@vmarchaud vmarchaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dyladan dyladan merged commit ce6a5eb into open-telemetry:master Mar 30, 2020
@dyladan dyladan deleted the tracer-no-default branch March 30, 2020 17:31
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
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.

6 participants