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

[cmake] Optional C bindings #1947

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

DownerCase
Copy link
Contributor

Description

Adds a CMake option for including the C language bindings.
I know this might be contentious, as I've seen sentiment that you have too many CMake options already (personally, I think its ok and I hope you never look at VTK :laughing).

It feels like the C bindings are considered important, which is why I kept it default-on and marked as advanced to indicate it probably shouldn't be touched.
The motivation is that, for example, I don't need it for my Go bindings so would like to be to exclude it from the container build (actually I see there's a core_v5 target now which is a much larger unused resource for me...).

If the marked-as-advanced option is still too much, I ask at least for an undocumented regular variable.

Related issues

Cherry-pick to

@rex-schilasky rex-schilasky added the cherry-pick-to-NONE Don't cherry-pick these changes label Jan 26, 2025
@KerstinKeller
Copy link
Contributor

For me an additional option in this case is fine, now that the C bindings are properly split from the rest of eCAL core.

The extra C++ target was just a way to try and deal with all the API changes for eCAL 6 for our users. However in it's current state, it is not so much of a benefit.
We might even remove it again before the eCAL 6 release, or we can also introduce an option here.
Not 100% sure.

@KerstinKeller KerstinKeller merged commit ce3d715 into eclipse-ecal:master Jan 27, 2025
12 of 14 checks passed
@DownerCase DownerCase deleted the optional-c-bindings branch January 27, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-to-NONE Don't cherry-pick these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants