ORC-1639: [C++] Simplify flags in CMake and fix corresponding compiling errors - Githubissues

apache / orc

Apache ORC - the smallest, fastest columnar storage for Hadoop workloads

https://orc.apache.org/

Apache License 2.0

670 stars 477 forks source link

ORC-1639: [C++] Simplify flags in CMake and fix corresponding compiling errors #1826

Closed wgtmac closed 4 months ago

wgtmac commented 4 months ago

What changes were proposed in this pull request?

Remove unnecessary compiler flags and replace -Weverything with -Wall.
Fix warnings and errors that emerge after the flag change.

Why are the changes needed?

The C++ library has added -Weverything which results in a bunch of -Wno-xxx flags. We should reduce them as many as possible.

How was this patch tested?

Tested it locally with clang13, clang17 and gcc13.
Make sure all CIs are passed.

Was this patch authored or co-authored using generative AI tooling?

No.

dongjoon-hyun commented 4 months ago

BTW, @wgtmac , for the following PR description, please note that our CI has been running Docker tests already (Debian11/Debian24/Ubuntu24/Fedora37/OracleLinux9)

TODO: verify all the docker images.

Screenshot 2024-02-27 at 00 15 15

wgtmac commented 4 months ago

BTW, @wgtmac , for the following PR description, please note that our CI has been running Docker tests already (Debian11/Debian24/Ubuntu24/Fedora37/OracleLinux9)

Sorry for missing that!

dongjoon-hyun commented 4 months ago

No problem at all.

Sorry for missing that!

BTW, we added Apple Silicon test coverage too.

Screenshot 2024-02-27 at 07 58 51

wgtmac commented 4 months ago

What about the current title? @dongjoon-hyun

dongjoon-hyun commented 4 months ago

Looks nice!

dongjoon-hyun commented 4 months ago

Thank you for update, @wgtmac .

dongjoon-hyun commented 4 months ago

Merged to main.

dongjoon-hyun commented 4 months ago

Could you make a backporting PR to branch-2.0, @wgtmac ?

wgtmac commented 4 months ago

Thanks @dongjoon-hyun!

Opened https://github.com/apache/orc/pull/1828