-
Notifications
You must be signed in to change notification settings - Fork 40
Fix CMake presets not working with versions older than 3.28 #135
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
base: main
Are you sure you want to change the base?
Conversation
This was preventing the presets from being used with older versions of CMake which they'd work with otherwise.
6bd55f5 to
a26ae97
Compare
ConnorClancyDeakin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
The code successfully allows for earlier versions of cmake without inhibiting later versions as well as clears up what can be used with the changes to display names.
Type of Change
The change is allowing earlier versions of cmake to be changed specifically down to version 3.21 according to cmake documentation
Code Readability
The code is clear when read alongside cmake documentation to clear up things such as what versions the version number refers to
Maintainability
This is maintanable as it stays in line with what a cmake preset file is as well as improving it by making it clearer on things such as linux or wsl so people know wsl can be used
Code Simplicity
The code is not unnecasserily long it just does what it has to to fulfill its purpase
Edge Cases
this change enables more versions to be used for cmake so it should make for less problems enabling more people to use it.
Test Thoroughness
Tested with cmake version 4.0.3 and version 3.21 both versions were successful in building the project and tests runned as normal
Backward Compatibility
the code doesnt break modern versions of cmake as tested when version 4.0.3 was used
Performance Considerations
Performance worked well the changes shouldn't effect the performance
Security Concerns
this code doesnt seem to have any security concerns, it does allow for older versions of cmake but from my research i was unable to find any evidence that showed older versions of cmake to 3.21 has any security issues.
Dependencies
There are no new added dependencies.
Documentation
The documentation is simple enough to understand with changes to display names just making things clearer
Broccoli811
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR looks good to me. Lowering the preset schema version to 3 is a sensible fix that restores compatibility with older CMake installations (3.21+), which is important for students using older Linux/WSL environments.
The renaming of the Linux and Windows presets also improves clarity and helps prevent common confusion between WSL, native Linux, and MSYS toolchains. The change is low-risk, maintains full functionality, and aligns with how the presets are actually used.
I have tested with cmake version 4.1.0 and version 3.21 both versions were successful in building the project and the test cases passed successfully.
As a future improvement, it may be worth adding a short comment in the presets file explaining why the version is intentionally kept low, and possibly introducing a shared base preset later to reduce repetition. Overall, this is a solid and student-friendly update
approved!
Description
Lowered the version in the presets file to 3.
This fixes an issue I had while revising the unit testing guide where a high version number was preventing the presets from being used with older versions of CMake which they'd work with otherwise.
I also renamed the Linux preset to Linux/WSL to make it clear that it can also be used by WSL users, and renamed Windows to Windows (MSYS) to emphasise the difference.
Type of change
How Has This Been Tested?
Testing Checklist
The CMake build process runs as before, this just allows presets to be loaded with older versions of CMake (back to 3.21, per documentation)
Checklist