Skip to content

[#1425] Add missing getters for PublishSubscribe Publisher#1428

Open
maxence-d wants to merge 9 commits intoeclipse-iceoryx:mainfrom
maxence-d:iox2-1245-add-missing-getters-for-pubsub-publisher
Open

[#1425] Add missing getters for PublishSubscribe Publisher#1428
maxence-d wants to merge 9 commits intoeclipse-iceoryx:mainfrom
maxence-d:iox2-1245-add-missing-getters-for-pubsub-publisher

Conversation

@maxence-d
Copy link
Copy Markdown

@maxence-d maxence-d commented Mar 13, 2026

Notes for Reviewer

Adds allocation_strategy() and max_loaned_samples() getters to Publisher of PublishSubscribe services.
The code re-uses the same logic as other existing getters.

Pre-Review Checklist for the PR Author

  • Add sensible notes for the reviewer
  • PR title is short, expressive and meaningful
  • Consider switching the PR to a draft (Convert to draft)
    • as draft PR, the CI will be skipped for pushes
  • Relevant issues are linked in the References section
  • Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  • Commits messages are according to this guideline
    • Commit messages have the issue ID ([#123] Add posix ipc example)
    • Keep in mind to use the same email that was used to sign the Eclipse Contributor Agreement
  • Tests follow the best practice for testing
  • Changelog updated in the unreleased section including API breaking changes
  • Assign PR to reviewer
  • All checks have passed (except task-list-completed)

PR Reviewer Reminders

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

References

Closes #1245

@maxence-d maxence-d marked this pull request as ready for review March 13, 2026 04:51
Copy link
Copy Markdown
Member

@elBoberido elBoberido 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 overall. Just some formatting issues. Could you also try to add these new functions to the Python bindings?

@maxence-d maxence-d force-pushed the iox2-1245-add-missing-getters-for-pubsub-publisher branch from 25951b0 to ba04cf0 Compare March 16, 2026 08:41
@maxence-d maxence-d force-pushed the iox2-1245-add-missing-getters-for-pubsub-publisher branch from ba04cf0 to bb7bac0 Compare March 16, 2026 08:46
@elfenpiff
Copy link
Copy Markdown
Contributor

@maxence-d, great to see your first, of hopefully many, pull requests!

@maxence-d maxence-d requested a review from elBoberido April 3, 2026 04:18
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.43%. Comparing base (c94534d) to head (a164681).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
iceoryx2-cxx/include/iox2/enum_translation.hpp 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1428      +/-   ##
==========================================
- Coverage   77.45%   77.43%   -0.03%     
==========================================
  Files         419      419              
  Lines       39349    39375      +26     
  Branches     1284     1285       +1     
==========================================
+ Hits        30479    30489      +10     
- Misses       7833     7847      +14     
- Partials     1037     1039       +2     
Flag Coverage Δ
CPP 67.30% <85.71%> (+0.06%) ⬆️
Rust 77.28% <92.30%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
iceoryx2-cxx/include/iox2/publisher.hpp 67.10% <100.00%> (+1.82%) ⬆️
iceoryx2/src/port/publisher.rs 92.96% <100.00%> (+0.25%) ⬆️
iceoryx2-cxx/include/iox2/enum_translation.hpp 48.47% <80.00%> (+0.40%) ⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxence-d
Copy link
Copy Markdown
Author

I just saw the coverage values
will add tests

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.

3 participants