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: Avoid deprecation warnings in CMakeLists.txt by requiring minimum version 3.5 #302

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

simonhyll
Copy link
Contributor

@simonhyll simonhyll commented Oct 16, 2024

🤔 What's changed?

CMake below 3.5 is going to be deprecated. Setting the minimum version to 3.5 suppresses that annoying warning. Also, the minimum version should be set before the project, otherwise you also get an annoying warning about that.

⚡️ What's your motivation?

Warnings are annoying when they're this simple to fix.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

<3.5 CMake will be deprecated soon, and the version should be set before the project in order to avoid a warning
@olleolleolle olleolleolle changed the title fix: CMakeLists.txt fix: Avoid deprecation warnings in CMakeLists.txt by requiring minimum version 3.5 Oct 16, 2024
@olleolleolle
Copy link
Contributor

In order to learn how old the 3.5 version was, I had to visit the CMake website.

Here are the release notes for 3.5
https://cmake.org/cmake/help/latest/release/3.5.html

There have been 26 versions since that one.
https://cmake.org/cmake/help/latest/release/index.html

They didn't include any very clear dates in the release notes.

@luke-hill
Copy link
Contributor

I don't know enough about c to make an informed decision on this. But do you think this should be communicated in changelog @simonhyll

Also is this a runtime change? If so should it go into a minor release? Or is this something that would have to be considered a breaking change.

@ursfassler are you still taking an interest in the c project?

@ursfassler
Copy link

@luke-hill I am interested in having a working C/C++ cucumber implementation, but didn't knew/monitor this c code (I am on https://github.com/cucumber/cucumber-cpp/)
As I didn't had much time for it to, I am not up to date of the plans for the cucumber C/C++ implementation.

@ursfassler
Copy link

CMake 3.5 seems already ancient.
The change should not have an impact on the code, but it could break a build (i.e. may need some manual changes to fix the build).

@luke-hill
Copy link
Contributor

I'll wait to merge until you have more understanding and agree this is the way forward. As the next release is just going to be 30.0.1

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Nov 13, 2024

This looks fairly harmless.

And given that we currently don't have a dedicated maintainer for C and given that we do have a pull request that solve a problem for someone using Gherkin C, I think don't think it is a good idea to wait much longer. I reckon we can merge this. If it turns out to be a problem, I'm sure someone will open a new issue. 😄

@mpkorstanje mpkorstanje merged commit 65997bc into cucumber:main Nov 13, 2024
2 checks passed
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.

5 participants