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

[18259] Calling old on_participant_discovery overload #3479

Merged
merged 3 commits into from
Jun 22, 2023
Merged

[18259] Calling old on_participant_discovery overload #3479

merged 3 commits into from
Jun 22, 2023

Conversation

MiguelCompany
Copy link
Member

@MiguelCompany MiguelCompany commented Apr 21, 2023 •
edited by JLBuenoLopez
Loading

Description

This should fix #3460 by calling the old overload whenever the new one does not ignore the participant.

Please note that a test that mixes two versions of the product would be difficult to implement, so this fix should be manually checked.

The black-box tests currently use the old overload of the listener, so passing them is enough to check that the old overload is called when the new one has not been overridden.

No need to backport since it only affects 2.10.1.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • N/A 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.
  • Changes are ABI compatible.
  • Changes are API compatible.
  • N/A 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.
    Related documentation PR: [18259] Clarify participant discovery callbacks behavior Fast-DDS-docs#511
  • Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@MiguelCompany MiguelCompany added the doc-pending Issue or PR which is pending to be documented label Apr 21, 2023
@MiguelCompany MiguelCompany added this to the v2.10.2 milestone Apr 21, 2023
@MiguelCompany MiguelCompany mentioned this pull request Apr 21, 2023
Closed
1 task
@MiguelCompany MiguelCompany changed the title Calling old on_participant_discovery overload [18259] Calling old on_participant_discovery overload Apr 21, 2023
@JLBuenoLopez JLBuenoLopez modified the milestones: v2.10.2, v2.11.0 Jun 12, 2023
@JLBuenoLopez
Copy link
Contributor

@richiprosima please test this

@JLBuenoLopez JLBuenoLopez added the ci-pending PR which CI is running label Jun 13, 2023
JLBuenoLopez
JLBuenoLopez previously approved these changes Jun 13, 2023
View reviewed changes
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez left a comment

Choose a reason for hiding this comment

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

I think this should be backported to 2.10.x

@JLBuenoLopez JLBuenoLopez added versions-pending and removed ci-pending PR which CI is running labels Jun 14, 2023
@JLBuenoLopez
Copy link
Contributor

@Mergifyio backport 2.10.x

@mergify
Copy link
Contributor

mergify bot commented Jun 20, 2023 •
edited
Loading

backport 2.10.x

✅ Backports have been created

@JLBuenoLopez
Copy link
Contributor

@richiprosima please test this

@JLBuenoLopez JLBuenoLopez added ci-pending PR which CI is running and removed doc-pending Issue or PR which is pending to be documented versions-pending labels Jun 20, 2023
@JLBuenoLopez
Copy link
Contributor

@richiprosima please test this

@EduPonz EduPonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. ci-pending PR which CI is running and removed ci-pending PR which CI is running ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. labels Jun 21, 2023
@EduPonz
Copy link

EduPonz commented Jun 21, 2023

@richiprosima please test windows

@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 Jun 22, 2023
@EduPonz EduPonz merged commit f15c7c7 into master Jun 22, 2023
@EduPonz EduPonz deleted the bugfix/18257 branch June 22, 2023 05:05
mergify bot pushed a commit that referenced this pull request Jun 22, 2023
@MiguelCompany @mergify
Calling old on_participant_discovery overload (#3479)
9664f53 
* Refs #18257. Notify old overload when participant is not ignored.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18257. Change default implementation of new overload.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18259: update versions.md

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Co-authored-by: JLBuenoLopez-eProsima <[email protected]>
(cherry picked from commit f15c7c7)

# Conflicts:
#	versions.md
@mergify mergify bot mentioned this pull request Jun 22, 2023
Merged
EduPonz pushed a commit that referenced this pull request Aug 7, 2023
@MiguelCompany @EduPonz
Calling old on_participant_discovery overload (#3479)
502926e 
* Refs #18257. Notify old overload when participant is not ignored.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18257. Change default implementation of new overload.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18259: update versions.md

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Co-authored-by: JLBuenoLopez-eProsima <[email protected]>
(cherry picked from commit f15c7c7)

# Conflicts:
#	versions.md
EduPonz added a commit that referenced this pull request Aug 9, 2023
@mergify @MiguelCompany @EduPonz
Calling old on_participant_discovery overload (#3479) (#3613)
4fb0d48 
* Calling old `on_participant_discovery` overload (#3479)

* Refs #18257. Notify old overload when participant is not ignored.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18257. Change default implementation of new overload.

Signed-off-by: Miguel Company <[email protected]>

* Refs #18259: update versions.md

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Co-authored-by: JLBuenoLopez-eProsima <[email protected]>
(cherry picked from commit f15c7c7)

# Conflicts:
#	versions.md

* Refs #18259: Fix rebase

Signed-off-by: Eduardo Ponz <[email protected]>

* Update versions.md

Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Eduardo Ponz <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
Co-authored-by: Eduardo Ponz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EduPonz EduPonz EduPonz approved these changes

@JLBuenoLopez JLBuenoLopez JLBuenoLopez left review comments

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.11.0
Development

Successfully merging this pull request may close these issues.

[18257] Version compatibility issue for participant discovery when upgrading to version 2.10.1
3 participants
@MiguelCompany @JLBuenoLopez @EduPonz