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.

Sign up for GitHub

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

Jump to bottom

[20742] Run is_plain method with the corresponding data representation #4681

Merged
merged 10 commits into from
May 8, 2024
Merged

[20742] Run is_plain method with the corresponding data representation #4681

merged 10 commits into from
May 8, 2024

Conversation

JesusPoderoso
Copy link
Contributor

@JesusPoderoso JesusPoderoso commented Apr 10, 2024 •
edited by EduPonz
Loading

Description

The endpoints implementation (DataReaderImpl and DataWriterImpl) were calling is_plain method without the data representation, so it was not being considered.
This PR includes the data representation as an argument to is_plain so it is considered in both cases.

@Mergifyio backport 2.13.x

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • N/A Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • Changes are ABI compatible.
  • Changes are API compatible.
  • N/A New feature has been added to the versions.md file (if applicable).
  • N/A New feature has been documented/Current behavior is correctly described in the documentation.
  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@JesusPoderoso JesusPoderoso added this to the v2.14.1 milestone Apr 10, 2024
@JesusPoderoso JesusPoderoso added the conflicts Backport PR wich git cherry pick failed label Apr 11, 2024
@JesusPoderoso JesusPoderoso force-pushed the hotfix/isplain_datarepresentation branch from d2b0d1e to fed22b6 Compare April 18, 2024 11:00
@JesusPoderoso JesusPoderoso removed the conflicts Backport PR wich git cherry pick failed label Apr 18, 2024
@github-actions github-actions bot added the ci-pending PR which CI is running label Apr 18, 2024
@JesusPoderoso JesusPoderoso requested review from MiguelCompany and removed request for MiguelCompany April 21, 2024 21:24
@EduPonz EduPonz added the to-do label Apr 23, 2024
@JesusPoderoso
Refs #20742: Add regression tests for endpoints
8aa7a59 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20742: Run method with the corresponding data representation
19c46a0 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20742: Set SampleLoanManager variable with property value
e7a49f5 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20574: Please linter
5dcd4a5 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20742: Check is_plain for all possible DataReader's data repr…
6411213 
…esentation

Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20742: Fix failures in other tests
fdf5b40 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso
Refs #20742: Fix unset datarepresentation case
1db708d 
Signed-off-by: JesusPoderoso <[email protected]>
@JesusPoderoso JesusPoderoso force-pushed the hotfix/isplain_datarepresentation branch from dc47bb2 to 1db708d Compare April 25, 2024 13:08
@JesusPoderoso JesusPoderoso requested review from MiguelCompany and removed request for MiguelCompany April 25, 2024 13:08
@JesusPoderoso JesusPoderoso requested review from MiguelCompany and removed request for MiguelCompany April 26, 2024 07:49
@JesusPoderoso
Copy link
Contributor Author

@richiprosima please test this

MiguelCompany
MiguelCompany requested changes May 7, 2024
View reviewed changes
src/cpp/fastdds/subscriber/DataReaderImpl.cpp Outdated Show resolved Hide resolved
test/unittest/dds/publisher/DataWriterTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/publisher/DataWriterTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/publisher/DataWriterTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/subscriber/DataReaderTests.cpp Outdated Show resolved Hide resolved
test/unittest/dds/subscriber/DataReaderTests.cpp Show resolved Hide resolved
MiguelCompany
MiguelCompany previously approved these changes May 7, 2024
View reviewed changes
@JesusPoderoso
Refs #20742: Please linter
ed59867 
Signed-off-by: JesusPoderoso <[email protected]>
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.13.x

@mergify Mergify
Copy link
Contributor

mergify bot commented May 7, 2024 •
edited
Loading

backport 2.13.x

✅ Backports have been created

@MiguelCompany
Copy link
Member

@richiprosima Please test this

@EduPonz EduPonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels May 8, 2024
@EduPonz EduPonz merged commit aef4f43 into master May 8, 2024
11 of 17 checks passed
@EduPonz EduPonz deleted the hotfix/isplain_datarepresentation branch May 8, 2024 08:42
mergify bot pushed a commit that referenced this pull request May 8, 2024
@JesusPoderoso @mergify
Run is_plain method with the corresponding data representation (#4681)
85ee253 
* Refs #20742: Add regression tests for endpoints

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Run  method with the corresponding data representation

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Set SampleLoanManager variable with property value

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20574: Please linter

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Check `is_plain` for all possible DataReader's data representation

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Fix failures in other tests

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Fix unset datarepresentation case

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Add Miguel' suggestion

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Apply rev suggestions

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Please linter

Signed-off-by: JesusPoderoso <[email protected]>

---------

Signed-off-by: JesusPoderoso <[email protected]>
(cherry picked from commit aef4f43)
@mergify mergify bot mentioned this pull request May 8, 2024
Merged
11 tasks
This was referenced May 8, 2024
Merged
Merged
@mergify mergify bot mentioned this pull request May 9, 2024
Merged
EduPonz pushed a commit that referenced this pull request May 9, 2024
@mergify @JesusPoderoso
Run is_plain method with the corresponding data representation (#4681…
565890a 
…) (#4757)

* Refs #20742: Add regression tests for endpoints

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Run  method with the corresponding data representation

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Set SampleLoanManager variable with property value

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20574: Please linter

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Check `is_plain` for all possible DataReader's data representation

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Fix failures in other tests

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Fix unset datarepresentation case

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Add Miguel' suggestion

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Apply rev suggestions

Signed-off-by: JesusPoderoso <[email protected]>

* Refs #20742: Please linter

Signed-off-by: JesusPoderoso <[email protected]>

---------

Signed-off-by: JesusPoderoso <[email protected]>
(cherry picked from commit aef4f43)

Co-authored-by: Jesús Poderoso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany approved these changes

Assignees
No one assigned
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.14.1
Development

Successfully merging this pull request may close these issues.
3 participants
@JesusPoderoso @MiguelCompany @EduPonz