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

[20583] DataReader::return_loan returns OK on loanable sequences without loans #4503

Merged
merged 4 commits into from
Apr 24, 2024
Merged

[20583] DataReader::return_loan returns OK on loanable sequences without loans #4503

merged 4 commits into from
Apr 24, 2024

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Mar 5, 2024 •
edited
Loading

Description

With this PR, DataReader::return_loan starts returning OK when given a loanable sequence with no loans, i.e.;

  1. A loanable sequence with a set maximum (no loans are performed)
  2. A empty loanable sequence

IMPORTANT: Even though these changes are API and ABI compatible, this a behavioral change and cannot target a patch release.

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
  • Any new/modified methods have been properly documented using Doxygen.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
  • New feature has been documented/Current behavior is correctly described in the documentation.
  • N/A; 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.

@EduPonz EduPonz added the needs-review PR that is ready to be reviewed label Mar 5, 2024
@EduPonz EduPonz added this to the v3.0.0 milestone Mar 5, 2024
@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from 3023499 to 70c3e45 Compare March 5, 2024 13:43
@JLBuenoLopez JLBuenoLopez force-pushed the 3.0.x-devel branch 5 times, most recently from ba2327e to eb65c57 Compare March 8, 2024 06:45
Mario-DL
Mario-DL approved these changes Mar 11, 2024
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes (last 4 commits) LGTM with green CI (would need rebase first)

@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from 70c3e45 to cd977f9 Compare March 12, 2024 08:44
@EduPonz
Copy link
Author

EduPonz commented Mar 12, 2024

@richiprosima please test_3 this

@EduPonz EduPonz added ci-pending PR which CI is running and removed needs-review PR that is ready to be reviewed labels Mar 12, 2024
@EduPonz EduPonz force-pushed the 3.0.x-devel branch 3 times, most recently from c899270 to a0bdc0e Compare March 25, 2024 12:01
@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from cd977f9 to d28468a Compare March 26, 2024 06:48
@EduPonz EduPonz requested a review from Mario-DL March 27, 2024 06:11
@EduPonz
Copy link
Author

EduPonz commented Mar 27, 2024

@richiprosima please test_3 this

Mario-DL
Mario-DL approved these changes Apr 1, 2024
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, as soon as we rebase this time and run ci we should merge to avoid further conflicts

@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from d28468a to 9f26885 Compare April 2, 2024 05:22
@EduPonz EduPonz force-pushed the 3.0.x-devel branch 2 times, most recently from 8c0830d to 5148f5d Compare April 18, 2024 13:08
@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from 9f26885 to 7a9ec39 Compare April 18, 2024 13:53
@EduPonz EduPonz requested a review from Mario-DL April 18, 2024 13:53
@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from 7a9ec39 to a34ffde Compare April 18, 2024 14:02
@Mario-DL Mario-DL requested review from Mario-DL and removed request for Mario-DL April 19, 2024 06:13
Mario-DL
Mario-DL approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green CI

@EduPonz
Refs #20583: Add test and fix doxygen
7664b01 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20583: return_loan returns OK when the loanable collection has …
d7c67be 
…ownership (does not have loans)

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20583: Update tests to new behaviour
75c739f 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz
Refs #20583: Add entry to versions.md
5498c31 
Signed-off-by: EduPonz <eduardoponz@eprosima.com>
@EduPonz EduPonz force-pushed the feature/return_loan_empty_seqs branch from a34ffde to 5498c31 Compare April 23, 2024 10:39
@EduPonz EduPonz requested a review from Mario-DL April 23, 2024 10:39
Mario-DL
Mario-DL approved these changes Apr 23, 2024
View reviewed changes
Copy link
Member

@Mario-DL Mario-DL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with green Windows CI

@EduPonz EduPonz removed the ci-pending PR which CI is running label Apr 24, 2024
@EduPonz EduPonz merged commit 3c91d03 into 3.0.x-devel Apr 24, 2024
8 of 12 checks passed
@EduPonz EduPonz deleted the feature/return_loan_empty_seqs branch April 24, 2024 05:16
EduPonz added a commit that referenced this pull request May 8, 2024
@EduPonz
DataReader::return_loan returns OK on loanable sequences without loans (
8c2c3d7 
#4503)

* Refs #20583: Add test and fix doxygen

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: return_loan returns OK when the loanable collection has ownership (does not have loans)

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: Update tests to new behaviour

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: Add entry to versions.md

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

---------

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
EduPonz added a commit that referenced this pull request May 13, 2024
@EduPonz
DataReader::return_loan returns OK on loanable sequences without loans (
26fd7e5 
#4503)

* Refs #20583: Add test and fix doxygen

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: return_loan returns OK when the loanable collection has ownership (does not have loans)

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: Update tests to new behaviour

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

* Refs #20583: Add entry to versions.md

Signed-off-by: EduPonz <eduardoponz@eprosima.com>

---------

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mario-DL Mario-DL Mario-DL approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@EduPonz @Mario-DL