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 macros #1112

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

Sean-Der
Copy link
Contributor

@Sean-Der Sean-Der commented Feb 16, 2024

Add version.h with the following values. This values are managed/updated via CMake

#pragma once

#define RTC_VERSION_MAJOR 0
#define RTC_VERSION_MINOR 20
#define RTC_VERSION_PATCH 1
#define RTC_VERSION "0.20.1"

Resolves #1111

Copy link
Owner

@paullouisageneau paullouisageneau left a 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?

@Sean-Der
Copy link
Contributor Author

Sean-Der commented Feb 19, 2024

@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

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"
```
@Sean-Der
Copy link
Contributor Author

Done! @paullouisageneau can I get another review?

Copy link
Owner

@paullouisageneau paullouisageneau 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, thank you!

@paullouisageneau paullouisageneau merged commit d498c84 into paullouisageneau:master Feb 21, 2024
12 checks passed
@Sean-Der Sean-Der deleted the version-macro branch February 28, 2024 16:03
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.

Add version macros
2 participants