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

Check required Conan version #517

Merged
merged 11 commits into from
May 21, 2023

Conversation

ssrobins
Copy link
Contributor

@ssrobins ssrobins commented May 18, 2023

This adds a Conan version check so people don't get stuck trying to get this to work on an unsupported Conan version. Recently discussed in #512.

In addition to adding this functionality, I set up a rudimentary cmake unit test layer to validate Conan version parsing and version check scenarios. I see there's already some extensive test changes going on in #515, but I decided not to base this branch on that because it would make the diff harder to read. I did locate the tests in a tests subdir to try to follow the pattern from that pull request.

Look at the previous checks to see temporary breaks in the minimum version and the unit tests to prove they work and fail GitHub Actions.

Anyway, this is my first contribution to the repo so I'm still learning. As always, I'm open to changes to make it better and conform to the codebase!

@CLAassistant
Copy link

CLAassistant commented May 18, 2023

CLA assistant check
All committers have signed the CLA.

@ssrobins ssrobins marked this pull request as ready for review May 18, 2023 06:51
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contributioN!

.github/workflows/cmake_conan.yml Outdated Show resolved Hide resolved
conan_support.cmake Outdated Show resolved Hide resolved
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments, thanks!

conan_support.cmake Outdated Show resolved Hide resolved
conan_support.cmake Outdated Show resolved Hide resolved
conan_support.cmake Outdated Show resolved Hide resolved
conan_support.cmake Outdated Show resolved Hide resolved
@ssrobins
Copy link
Contributor Author

Thanks for the feedback, I think it resulted in great improvements. Let me know if you see anything else!

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks very much

@ssrobins ssrobins force-pushed the check-conan-version branch from f0e0788 to 7d39630 Compare May 19, 2023 23:57
@memsharded memsharded merged commit f49e229 into conan-io:develop2 May 21, 2023
@memsharded
Copy link
Member

Merged, thanks for your contribution!

@ssrobins
Copy link
Contributor Author

Merged, thanks for your contribution!

Thanks!! I can squash things down a bit next time, wasn't sure if that was going to be automatic or not.

@ssrobins ssrobins deleted the check-conan-version branch May 21, 2023 16:23
@memsharded
Copy link
Member

Thanks!! I can squash things down a bit next time, wasn't sure if that was going to be automatic or not.

Not necessary, it was my bad. I usually have the "Squash & merge" Github button activated, but this time I didn't realize it was just merging without squash.

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