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

Add version number in header files #459

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

marcalff
Copy link
Contributor

@marcalff marcalff commented Aug 3, 2024

Fixes #458

Changes

  • Add version numbers in header files, that can be used at compile time
  • Add functions to get the library version numbers, that can be used at runtime. This helps with shared libraries.

@biojppm
Copy link
Owner

biojppm commented Aug 10, 2024

Thanks, this is appreciated!

The PR looks good, but there is one extra thing, though: add corresponding entries for the new files in the tbump file.

@marcalff
Copy link
Contributor Author

Thanks, this is appreciated!

The PR looks good, but there is one extra thing, though: add corresponding entries for the new files in the tbump file.

Interesting tool, will look into this.

@marcalff
Copy link
Contributor Author

Thanks, this is appreciated!
The PR looks good, but there is one extra thing, though: add corresponding entries for the new files in the tbump file.

Interesting tool, will look into this.

Done

@biojppm
Copy link
Owner

biojppm commented Aug 10, 2024

Thanks! I've added some tests; let's wait for the pipelines to turn green.

@marcalff
Copy link
Contributor Author

marcalff commented Aug 10, 2024

CI is a carnage, not sure why in most cases.

Found this for find_package()

/home/runner/work/rapidyaml/rapidyaml/prefix/find_package/linux-Debug/include/c4/yml/yml.hpp:4:10: fatal error: c4/yml/version.hpp: No such file or directory

Is there a place to update for new headers to install ?


May need this:

[malff@malff-desktop rapidyaml]$ git diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f2f2557..ef7efaf 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -70,6 +70,7 @@ c4_add_library(ryml
         c4/yml/tag.cpp
         c4/yml/tree.hpp
         c4/yml/tree.cpp
+        c4/yml/version.hpp
         c4/yml/version.cpp
         c4/yml/writer.hpp
         c4/yml/yml.hpp

@biojppm
Copy link
Owner

biojppm commented Aug 10, 2024

Indeed, that's missing.

Btw, the sea of red is unrelated; it's happening because GitHub updated nodejs in the workflow actions and the ubuntu18 runner is therefore no longer supported. Let's ignore this problem. Let's focus on the workflows related to install.

@biojppm
Copy link
Owner

biojppm commented Aug 13, 2024

@marcalff warning: I've rebased your branch into the current master.

@marcalff
Copy link
Contributor Author

@marcalff warning: I've rebased your branch into the current master.

That is fine, I noticed all the CI cleanup required as pre requisite.

I take it this PR is to be merged after #461 is done ?

@biojppm
Copy link
Owner

biojppm commented Aug 13, 2024

I take it this PR is to be merged after #461 is done ?

Well, by coincidence, not by necessity.


I've looked through the pipelines, and the relevant ones are all green (the install/release pipelines succeed, and the sample pipelines succeed for the relevant tests, failing later only because of an unrelated problem which happens with PRs of non-owners). So I'm merging now.

Thanks for your PR, and for sticking through this.

@biojppm biojppm merged commit 0b761bc into biojppm:master Aug 13, 2024
145 of 241 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.

Request: Add version number in header files
2 participants