Skip to content

Use only UPPERCASE CMake build types and check for it#12880

Closed
davidrohr wants to merge 1 commit intoAliceO2Group:devfrom
davidrohr:dev_pull_request5
Closed

Use only UPPERCASE CMake build types and check for it#12880
davidrohr wants to merge 1 commit intoAliceO2Group:devfrom
davidrohr:dev_pull_request5

Conversation

@davidrohr
Copy link
Collaborator

@ktf @pzhristov : @mconcas and me figured out today that compile flags are not set correctly for non-uppercase build types.

Don't know if we want to debug and try to fix it, or just request the build type to be uppercase, which is done in this PR. What do you think?

@davidrohr davidrohr requested review from a team, sgorbuno, shahor02 and wiechula as code owners March 18, 2024 17:04
@github-actions
Copy link
Contributor

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass
async-2023-pp-apass1
async-2022-pp-apass6
async-2022-pp-apass4
async-mc
async-data

@ktf
Copy link
Member

ktf commented Mar 19, 2024

For the record, in principle whole case sensitivity issue is described here:

https://cmake.org/cmake/help/v3.29/manual/cmake-buildsystem.7.html#case-sensitivity

and the "strong" convention is described a few lines below:

https://cmake.org/cmake/help/v3.29/manual/cmake-buildsystem.7.html#id38

I would personally prefer we stick to the convention and fix the cases where the wrong casing is used.

@ktf
Copy link
Member

ktf commented Mar 19, 2024

Or if you prefer, if I understand correctly, the "safe" pattern is what is done in #12886

@ktf
Copy link
Member

ktf commented Mar 19, 2024

As discussed, let's use #12886

@ktf ktf closed this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants