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

[#7531,#7532] Bump version number and roll our own CMake package version file (main) #7535

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

@SwooshyCueb
Copy link
Member Author

This PR and the listed companion PRs are ready for review

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Seems correct. I didn't check everything.

Are there tools that help with testing things like this?
Does it make since to introduce some type of testing mechanism for this?

cmake/IRODSConfigVersion.cmake.in Show resolved Hide resolved
cmake/IRODSConfigVersion.cmake.in Show resolved Hide resolved
cmake/IRODSConfigVersion.cmake.in Outdated Show resolved Hide resolved
@korydraughn
Copy link
Collaborator

Please resolve the clang-format report too.

@SwooshyCueb
Copy link
Member Author

Are there tools that help with testing things like this?

None that I know of. I did some basic testing myself by commenting out some set calls and calling the cmake file directly while setting variables on the command line, and I got the results I expected for every case I could think to test.

Does it make since to introduce some type of testing mechanism for this?

Only if we end up changing the version file more than once in a blue moon, which I don't see happening.

@korydraughn
Copy link
Collaborator

I'm kind of surprised there aren't tools for testing cmake code. Sounds like an opportunity to help the c/c++ ecosystem.

Anyway, happy to hear the version file won't need to be touched often.

@SwooshyCueb
Copy link
Member Author

I'm kind of surprised there aren't tools for testing cmake code. Sounds like an opportunity to help the c/c++ ecosystem.

Not so much that there aren't tools for testing CMake code (I think there are) but that there aren't any tools (that I know of) for testing input files that generate CMake code.

Copy link
Collaborator

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

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

Seems good/ready to me.

Let's get one more approval before pounding.

Copy link
Member

@trel trel left a comment

Choose a reason for hiding this comment

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

a couple things...

cmake/IRODSConfigVersion.cmake.in Outdated Show resolved Hide resolved
cmake/IRODSConfigVersion.cmake.in Outdated Show resolved Hide resolved
test/c_api_test/test.c Show resolved Hide resolved
Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

This is exciting stuff... Very nice comments in the IRODSConfigVersion file

cmake/IRODSConfigVersion.cmake.in Show resolved Hide resolved
cmake/IRODSConfigVersion.cmake.in Outdated Show resolved Hide resolved
cmake/IRODSConfigVersion.cmake.in Show resolved Hide resolved
test/c_api_test/test.c Show resolved Hide resolved
@alanking
Copy link
Contributor

alanking commented Mar 1, 2024

# it when ready

@SwooshyCueb
Copy link
Member Author

#'d

@alanking alanking merged commit f2315a7 into irods:main Mar 1, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants