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: Make multi-config presets work again #1670

Closed
wants to merge 1 commit into from
Closed

Conversation

jadebenn
Copy link
Collaborator

@jadebenn jadebenn commented Dec 8, 2024

Reverted breaking change for multiple-configuration presets that was previously included in #1669

Reverted breaking change for multiple-configuration presets that was previously included in #1669
@jadebenn
Copy link
Collaborator Author

jadebenn commented Dec 8, 2024

On further review, I'm not sure #1669 should have been merged at all. There are some changes that are counterproductive for handling multi-config generators correctly in CMake.

@Jettford
Copy link
Collaborator

Jettford commented Dec 8, 2024

I believe this PR is counter-productive to changes I am trying to make on the origin/windows-clang branch. Not closing as I am awaiting further feedback.

@Xiphoseer
Copy link
Contributor

Fwiw: I'm generally in favor of not tying things down to a build directory, because it doesn't even have to be called that in cmake.

At the same time, this is our own env var which shouldn't confuse tooling too much, so if it restores stuff that was working it may be fine to merge even if replaced by a better way later.

@EmosewaMC
Copy link
Collaborator

This makes it so CI no longer contains ini files. I would rather prioritize CI and release over development environments.

@jadebenn
Copy link
Collaborator Author

jadebenn commented Dec 8, 2024

closing - there is a better approach

@jadebenn jadebenn closed this Dec 8, 2024
@jadebenn jadebenn deleted the jadebenn-patch-1 branch December 8, 2024 20:42
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