Skip to content

[20592][20679] Fix hidden overloaded virtual methods#4625

Merged
EduPonz merged 1 commit into2.13.xfrom
hotfix/hidden_overloads/2.13.x
Mar 31, 2024
Merged

[20592][20679] Fix hidden overloaded virtual methods#4625
EduPonz merged 1 commit into2.13.xfrom
hotfix/hidden_overloads/2.13.x

Conversation

@EduPonz
Copy link
Copy Markdown

@EduPonz EduPonz commented Mar 26, 2024 •
edited by JesusPoderoso
Loading

Description

This PR is a backport combining:

Before this PR, compiling with GCC option -Woverloaded-virtual resulted in compilation warnings due to overloaded virtual methods being hidden in derived classes that do not override all the parent's overloads. This PR:

  1. Fixes all those warnings
  2. Adds more warning reporting options GCC in Ubuntu CI on Github
  3. Enables running Github Ubuntu CI on pull_request events
  4. Enables running Github CI on PRs targeting intermediate branches

It substitutes:

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).
  • N/A: 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 this to the v2.13.4 milestone Mar 26, 2024
This was referenced Mar 26, 2024
Closed
Closed
@JesusPoderoso
Copy link
Copy Markdown
Contributor

JesusPoderoso commented Mar 26, 2024

@richiprosima please test this

JesusPoderoso
JesusPoderoso previously approved these changes Mar 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

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

@JesusPoderoso JesusPoderoso added the ci-pending PR which CI is running label Mar 27, 2024
@JesusPoderoso
Copy link
Copy Markdown
Contributor

JesusPoderoso commented Mar 27, 2024

CI issues unrelated to the PR.
Ready to merge!

@JesusPoderoso JesusPoderoso 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 Mar 27, 2024
@EduPonz
Refs #20592: Fix hidden overloaded virtual methods
1ba569e 
* Refs #20592: Fix for test

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

* Refs #20592: Fix for examples

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

* Refs #20592: Add more warning flags to Ubuntu CI

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

* Refs #20592: Remove default values on overloaded PDPClient::announceParticipantState

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

---------

Signed-off-by: EduPonz <eduardoponz@eprosima.com>
(cherry picked from commit 63cc242)

Fix conflicts

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>
@EduPonz EduPonz force-pushed the hotfix/hidden_overloads/2.13.x branch from 5acfad9 to 1ba569e Compare March 28, 2024 09:00
@EduPonz EduPonz requested a review from JesusPoderoso March 28, 2024 09:00
@github-actions github-actions bot added ci-pending PR which CI is running labels Mar 28, 2024
@EduPonz EduPonz removed the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Mar 28, 2024
@JesusPoderoso
Copy link
Copy Markdown
Contributor

JesusPoderoso commented Mar 29, 2024

@richiprosima please test this

@JesusPoderoso
Copy link
Copy Markdown
Contributor

JesusPoderoso commented Mar 30, 2024

CI issues unrelated to the PR.
Ready to merge!

@JesusPoderoso JesusPoderoso 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 Mar 30, 2024
@EduPonz EduPonz merged commit eed4789 into 2.13.x Mar 31, 2024
@EduPonz EduPonz deleted the hotfix/hidden_overloads/2.13.x branch March 31, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@JesusPoderoso JesusPoderoso JesusPoderoso approved these changes

Reviewers whose approvals may not affect merge requirements

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.13.4

Development

Successfully merging this pull request may close these issues.

2 participants

@EduPonz @JesusPoderoso