-
-
Notifications
You must be signed in to change notification settings - Fork 371
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 macros #1112
Add version macros #1112
Conversation
9175e26
to
5149c4a
Compare
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.
It looks like the version.h
header is only put in place at installation, so the RTC_VERSION
defines are absent when adding the library to a project directly with add_subdirectory()
. There are also not available to the library itself and examples. This can probably be fixed by adding something like target_include_directories(libdatachannel[-static] PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>/include/rtc)
but relying on include directory search order seems a bit hacky.
Another issue is that the defines are also absent if the library is built with the Makefile or the Jamfile. It seems to me that common practice to support multiple build systems is a config.h
file that the different build systems configure but I would prefer not to go there, I like that the code compiles as-is for the sake of simplicity.
IMHO it would be better to hardcode the version in include/rtc/version.h
so it just works (current defines look good). I'll update them when releasing, and possibly read them from CMakeLists later so the version number is in a single place. What do you think?
@paullouisageneau i like that idea! I will make CMake write to version.h I will update and add a GitHub action to make sure it stays in sync. So build will fail of version isn’t updated properly |
5149c4a
to
faf858c
Compare
Add version.h with the following values. This values are managed/updated via CMake ``` #define RTC_VERSION_MAJOR 0 #define RTC_VERSION_MINOR 20 #define RTC_VERSION_PATCH 1 #define RTC_VERSION "0.20.1" ```
faf858c
to
3045e73
Compare
Done! @paullouisageneau can I get another review? |
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.
Looks good, thank you!
Add version.h with the following values. This values are managed/updated via CMake
Resolves #1111