eclipse-iceoryx / iceoryx

Eclipse iceoryx™ - true zero-copy inter-process-communication
https://iceoryx.io
Apache License 2.0
1.65k stars 384 forks source link

iox-#2327 Replace macro ENUM with IOX_ENUM and CLASS with IOX_CLASS #2328

Closed mikebentley15 closed 1 month ago

mikebentley15 commented 1 month ago

Notes for Reviewer

The macros ENUM and CLASS are very generic names and can potentially cause naming conflicts with user code or other third-party libraries. These macros are intended to make the headers compatible with both C and C++. The ENUM macro is unnecessary since the enum prefix for variable declarations is required in C and optional in C++. The CLASS macro is required to match the Microsoft C++ ABI -- defined as class when in C++ mode and struct when in C mode.

This PR seeks to solve this by

See issue #2327 for a minimal example to reproduce the issue and existing workarounds.

Pre-Review Checklist for the PR Author

  1. [x] Code follows the coding style of CONTRIBUTING.md
  2. [x] Tests follow the best practice for testing
  3. [x] Changelog updated in the unreleased section including API breaking changes
  4. [x] Branch follows the naming format (iox-123-this-is-a-branch)
  5. [x] Commits messages are according to this guideline
  6. [x] Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  7. [x] Relevant issues are linked
  8. [x] Add sensible notes for the reviewer
  9. [x] All checks have passed (except task-list-completed)
  10. [x] Assign PR to reviewer

Checklist for the PR Reviewer

Post-review Checklist for the PR Author

  1. [x] All open points are addressed and tracked via issues

References

mikebentley15 commented 1 month ago

After this review goes through, I may make another PR to redo this change for version v2.90 to integrate with our pipeline.

elBoberido commented 1 month ago

After this review goes through, I may make another PR to redo this change for version v2.90 to integrate with our pipeline.

There is no branch for v2.90. It is only a tag and therefore there is nothing a PR could be merged to. If you need this for your internal pipeline, you need to either carry a patch or update to the latest main branch.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.23%. Comparing base (912931a) to head (21f6d09). Report is 23 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328/graphs/tree.svg?width=650&height=150&src=pr&token=KWu8wdCc1S&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) ```diff @@ Coverage Diff @@ ## main #2328 +/- ## ========================================== + Coverage 78.07% 78.23% +0.15% ========================================== Files 432 433 +1 Lines 15920 16034 +114 Branches 2303 2302 -1 ========================================== + Hits 12430 12544 +114 Misses 2644 2644 Partials 846 846 ``` | [Flag](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | `78.05% <100.00%> (+0.15%)` | :arrow_up: | | [unittests_timing](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | `15.03% <33.33%> (+0.08%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx) | Coverage Δ | | |---|---|---| | [...ceoryx\_binding\_c/source/c2cpp\_enum\_translation.cpp](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?src=pr&el=tree&filepath=iceoryx_binding_c%2Fsource%2Fc2cpp_enum_translation.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#diff-aWNlb3J5eF9iaW5kaW5nX2Mvc291cmNlL2MyY3BwX2VudW1fdHJhbnNsYXRpb24uY3Bw) | `100.00% <100.00%> (ø)` | | | [iceoryx\_binding\_c/source/c\_listener.cpp](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?src=pr&el=tree&filepath=iceoryx_binding_c%2Fsource%2Fc_listener.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#diff-aWNlb3J5eF9iaW5kaW5nX2Mvc291cmNlL2NfbGlzdGVuZXIuY3Bw) | `85.45% <100.00%> (ø)` | | | [iceoryx\_binding\_c/source/c\_service\_discovery.cpp](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?src=pr&el=tree&filepath=iceoryx_binding_c%2Fsource%2Fc_service_discovery.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#diff-aWNlb3J5eF9iaW5kaW5nX2Mvc291cmNlL2Nfc2VydmljZV9kaXNjb3ZlcnkuY3Bw) | `98.27% <ø> (ø)` | | | [iceoryx\_binding\_c/source/c\_wait\_set.cpp](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328?src=pr&el=tree&filepath=iceoryx_binding_c%2Fsource%2Fc_wait_set.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx#diff-aWNlb3J5eF9iaW5kaW5nX2Mvc291cmNlL2Nfd2FpdF9zZXQuY3Bw) | `90.22% <100.00%> (ø)` | | ... and [16 files with indirect coverage changes](https://app.codecov.io/gh/eclipse-iceoryx/iceoryx/pull/2328/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eclipse-iceoryx)
elBoberido commented 1 month ago

@mikebentley15 thanks very much for your contribution. I think I already mentioned it above, we are currently working on iceoryx2 which is written in Rust and will have C and C++ bindings with the next release. We will also present it in the iceoryx developer meetup next week on Tuesday. We would also be happy to hear what you are using iceoryx for :)

mikebentley15 commented 1 month ago

@elBoberido, you're welcome! It's pretty cool that you have iceoryx2 implemented in Rust. I'm using iceoryx indirectly via Cyclone DDS. Cheers!