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] Fix old style cast warning #2567

Merged
merged 1 commit into from
Feb 28, 2024
Merged

[BUILD] Fix old style cast warning #2567

merged 1 commit into from
Feb 28, 2024

Conversation

keith
Copy link
Contributor

@keith keith commented Feb 28, 2024

Fixes #2556

@lalitb
Copy link
Member

lalitb commented Feb 28, 2024

Thanks for the fix. Would be also good to add this warning as an error in maintainer mode CI -

if(OTELCPP_MAINTAINER_MODE)
, this will reveal if there are other places to fix too.

@keith
Copy link
Contributor Author

keith commented Feb 28, 2024

There are actually quite a few more, I will push them and that cmake change here

@keith
Copy link
Contributor Author

keith commented Feb 28, 2024

pushed! note there are a few cases where reinterpret_cast was the replacement instead

@keith
Copy link
Contributor Author

keith commented Feb 28, 2024

Ok I think this is ready for re-review!

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.

A few more warnings indeed, thanks for the cleanup.

See a comment on SetCurlPtrOption

@marcalff marcalff changed the title Fix old style cast warning [BUILD] Fix old style cast warning Feb 28, 2024
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 cleanup.

@lalitb lalitb 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 Feb 28, 2024
@marcalff
Copy link
Member

@keith Last CI failures are in tests, because of ifdef for ABI_V2.

/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/test/trace/tracer_provider_test.cc:175:33: error: use of old-style cast [-Werror,-Wold-style-cast]
                          {"d", (unsigned int)314159},

Should be ready to go after this last pass.

@keith
Copy link
Contributor Author

keith commented Feb 28, 2024

ah thanks yea i didn't build with that locally, pushed

@marcalff
Copy link
Member

marcalff commented Feb 28, 2024

@keith Last CI failures are in tests, because of ifdef for ABI_V2.

/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/test/trace/tracer_provider_test.cc:175:33: error: use of old-style cast [-Werror,-Wold-style-cast]
                          {"d", (unsigned int)314159},

Should be ready to go after this last pass.

Search for "error:" in the CI logs.

Maintainer builds do a make -k to report as many errors as possible in one build, there are a few reported there.

Or build WITH_ABI_VERSION_2 locally (Cmake).

@marcalff marcalff merged commit eaaf6cd into open-telemetry:main Feb 28, 2024
47 checks passed
@keith keith deleted the ks/fix-old-style-cast-warning branch February 28, 2024 22:01
@keith
Copy link
Contributor Author

keith commented Feb 28, 2024

thanks!

@marcalff
Copy link
Member

Thanks @keith

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.

[BUILD] Old style cast warnings in opentelemetry_api headers
4 participants