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

[BUILD] Use fully qualified references to trace/common namespace #2424

Merged
merged 6 commits into from
Dec 5, 2023

Conversation

bogdandrutu
Copy link
Member

Fixes #2417

@bogdandrutu bogdandrutu force-pushed the fix_2417 branch 3 times, most recently from f711291 to 4a352b6 Compare December 2, 2023 00:56
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu
Copy link
Member Author

@ThomsonTan unfortunately on my mac format produces crazy results (maybe some hints on how to properly run would be appreciated), is it possible to help me with formatting this code?

Copy link

codecov bot commented Dec 2, 2023

Codecov Report

Merging #2424 (a7e482c) into main (39ad238) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2424      +/-   ##
==========================================
- Coverage   87.06%   87.04%   -0.01%     
==========================================
  Files         199      199              
  Lines        6079     6080       +1     
==========================================
  Hits         5292     5292              
- Misses        787      788       +1     
Files Coverage Δ
...pentelemetry/trace/span_context_kv_iterable_view.h 84.22% <ø> (ø)
...lude/opentelemetry/sdk/common/global_log_handler.h 72.23% <ø> (ø)
...y/sdk/instrumentationscope/instrumentation_scope.h 100.00% <100.00%> (ø)
...k/metrics/exemplar/fixed_size_exemplar_reservoir.h 71.43% <ø> (ø)
...dk/metrics/exemplar/histogram_exemplar_reservoir.h 81.25% <ø> (ø)
...ude/opentelemetry/sdk/metrics/exemplar/reservoir.h 100.00% <ø> (ø)
...entelemetry/sdk/metrics/state/attributes_hashmap.h 92.43% <ø> (ø)
sdk/include/opentelemetry/sdk/trace/span_data.h 98.67% <100.00%> (ø)
sdk/src/trace/span.h 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

@lalitb
Copy link
Member

lalitb commented Dec 2, 2023

@ThomsonTan unfortunately on my mac format produces crazy results (maybe some hints on how to properly run would be appreciated), is it possible to help me with formatting this code?

@bogdandrutu - You can do this by

@esigo
Copy link
Member

esigo commented Dec 3, 2023

@bogdandrutu you could use docker:

./ci/run_docker.sh  tools/format.sh

@bogdandrutu
Copy link
Member Author

@esigo that is awesome, but not documented :) Thanks

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

@marcalff marcalff changed the title Fix more references to trace/common namespace to be fully qualified [BUILD] Fix more references to trace/common namespace to be fully qualified Dec 4, 2023
@marcalff marcalff changed the title [BUILD] Fix more references to trace/common namespace to be fully qualified [BUILD] Use fully qualified references to trace/common namespace Dec 4, 2023
@marcalff marcalff added the ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve) label Dec 4, 2023
@marcalff marcalff mentioned this pull request Dec 4, 2023
Copy link
Member

@owent owent left a comment

Choose a reason for hiding this comment

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

LGTM after the format been fixed.

@marcalff
Copy link
Member

marcalff commented Dec 5, 2023

@bogdandrutu

@ThomsonTan unfortunately on my mac format produces crazy results (maybe some hints on how to properly run would be appreciated), is it possible to help me with formatting this code?

Applied clang-format-10 and pushed to your branch.

@marcalff marcalff merged commit d91e5ba into open-telemetry:main Dec 5, 2023
45 checks passed
@bogdandrutu bogdandrutu deleted the fix_2417 branch December 6, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'SpanContext' is not a member of 'opentelemetry::v1::sdk::trace'
6 participants