Skip to content

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

Merged
EduPonz merged 4 commits into3.0.x-develfrom
feature/return_loan_empty_seqs
Apr 24, 2024
Merged

[20583] DataReader::return_loan returns OK on loanable sequences without loans#4503
EduPonz merged 4 commits into3.0.x-develfrom
feature/return_loan_empty_seqs

Conversation

@EduPonz
Copy link
Copy Markdown

@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
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

@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 added 4 commits April 23, 2024 12:36
@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
Copy Markdown
Contributor

@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
@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

1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

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