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

fix_1558_cmake_build_type #1559

Merged
merged 1 commit into from
Apr 15, 2021
Merged

fix_1558_cmake_build_type #1559

merged 1 commit into from
Apr 15, 2021

Conversation

clanmills
Copy link
Collaborator

See: #1558.

@piponazo
Copy link
Collaborator

Hi Robin. In essence, this addition is good to have.

Nonetheless, I would propose to do something similar to what it is discussed in this blog post from kitware (the creators of CMake):
https://blog.kitware.com/cmake-and-the-default-build-type/

I also have one question: Do you have plans to include this in 0.27-maintenance, or that branch is freeze?

@kmilos
Copy link
Collaborator

kmilos commented Apr 15, 2021

There's also some more in the CMake FAQ, but I'm ok with this just minimal change, i.e. I don't know CMake well enough to say what's the best approach is and what all the extra modifiers do...

@heckflosse
Copy link
Collaborator

@cryptomilk is a cmake specialist. Perhaps he can have a look...

@clanmills clanmills merged commit 52b2855 into main Apr 15, 2021
@clanmills clanmills deleted the fix_1558_cmake_build_type branch April 15, 2021 12:23
@clanmills
Copy link
Collaborator Author

0.27-maintenance is frozen for now.

The code in both that CMake blog and FAQ makes me nervous. I think "When you're in a hole, stop digging.". I've brought the code into alignment with the documentation.

The error in the documentation is entirely mine. I believed that the default was Release - and now my guess has been shown to be wrong. As I mentioned on the chat server, I've never actually thought about this because CMAKE_BUILD_TYPE doesn't do anything on Visual Studio, CLion or Xcode.

It seems very unlikely that the build would default to Debug. I suspect another invisible agent is at work somewhere.

I notice that the person who opened the issue has made no comment about the resolution and not shown any appreciation for the amazing team response to his ticket.

@kmilos
Copy link
Collaborator

kmilos commented Nov 23, 2021

@mergify backport 0.27-maintenance

@mergify
Copy link
Contributor

mergify bot commented Nov 23, 2021

backport 0.27-maintenance

✅ Backports have been created

Hey, I reacted but my real name is @Mergifyio

clanmills added a commit that referenced this pull request Nov 25, 2021
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.

4 participants