iceoryx icon indicating copy to clipboard operation
iceoryx copied to clipboard

iox-#605 Merge `BaseRelativePointer` with `RelativePointer`

Open mossmaurice opened this issue 1 year ago • 2 comments

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
    • [x] Commit messages have the issue ID (iox-#123 commit text)
    • [x] Commit messages are signed (git commit -s)
    • [x] Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  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. [ ] All checks have passed (except task-list-completed)
  10. [x] All touched (C/C++) source code files from iceoryx_hoofs are added to ./clang-tidy-diff-scans.txt
  11. [x] Assign PR to reviewer

Notes for Reviewer

  • Merge BaseRelativePointer with RelativePointer
  • Introduce UntypedRelativePointer alias
  • Invalidate moved-from RelativePointer objects
  • Rename id_t to segment_id_t due to AUTOSAR rule M2.10.1

Checklist for the PR Reviewer

  • [ ] Commits are properly organized and messages are according to the guideline
  • [ ] Code according to our coding style and naming conventions
  • [ ] Unit tests have been written for new behavior
  • [ ] Public API changes are documented via doxygen
  • [ ] Copyright owner are updated in the changed files
  • [ ] All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
  • [ ] PR title describes the changes

Post-review Checklist for the PR Author

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

References

  • Closes #605 (partly)

mossmaurice avatar Sep 02 '22 10:09 mossmaurice

Codecov Report

Merging #1603 (5d77c8e) into master (8d2c793) will decrease coverage by 0.00%. The diff coverage is 89.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1603      +/-   ##
==========================================
- Coverage   76.21%   76.21%   -0.01%     
==========================================
  Files         366      365       -1     
  Lines       14163    14151      -12     
  Branches     2360     2361       +1     
==========================================
- Hits        10795    10785      -10     
+ Misses       2578     2576       -2     
  Partials      790      790              
Flag Coverage Δ
unittests 75.86% <89.07%> (-0.01%) :arrow_down:
unittests_timing 15.59% <21.84%> (+<0.01%) :arrow_up:

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

Impacted Files Coverage Δ
...nternal/relocatable_pointer/pointer_repository.hpp 100.00% <ø> (ø)
.../internal/relocatable_pointer/relative_pointer.hpp 100.00% <ø> (ø)
...de/iceoryx_posh/internal/roudi/process_manager.hpp 100.00% <ø> (ø)
...yx_posh/internal/runtime/ipc_runtime_interface.hpp 100.00% <ø> (ø)
iceoryx_posh/source/runtime/shared_memory_user.cpp 0.00% <0.00%> (ø)
...oryx_posh/source/runtime/ipc_runtime_interface.cpp 52.10% <50.00%> (ø)
.../internal/relocatable_pointer/relative_pointer.inl 96.29% <94.91%> (-3.71%) :arrow_down:
...nternal/relocatable_pointer/pointer_repository.inl 92.50% <100.00%> (-0.36%) :arrow_down:
...lude/iceoryx_posh/internal/mepoo/mepoo_segment.inl 88.88% <100.00%> (ø)
...posh/include/iceoryx_posh/internal/roudi/roudi.hpp 77.77% <100.00%> (ø)
... and 6 more

codecov[bot] avatar Sep 02 '22 10:09 codecov[bot]

@MatthiasKillat Seems there is still a problem somewhere:

  1. ~~The examples don't run~~ :heavy_check_mark:
  2. ~~MemoryProvider_Test.SegmentIdValueAfterCreationIsValid is failing on some builds~~ :heavy_check_mark:

You can have a look if you like. I won't be able to work on it in the next two weeks.

mossmaurice avatar Sep 02 '22 12:09 mossmaurice

@mossmaurice I will approve after the conflicts are resolved. Please change the move ctor in the follow up PR.

MatthiasKillat avatar Sep 28 '22 10:09 MatthiasKillat