Skip to content

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

Merged
EduPonz merged 1 commit into2.6.xfrom
hotfix/hidden_overloads/2.6.x
Apr 10, 2024
Merged

[20592][20679] Fix hidden overloaded virtual methods#4622
EduPonz merged 1 commit into2.6.xfrom
hotfix/hidden_overloads/2.6.x

Conversation

@EduPonz
Copy link
Copy Markdown

@EduPonz EduPonz commented Mar 26, 2024 •
edited
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.6.8 milestone Mar 26, 2024
This was referenced Mar 26, 2024
Closed
Closed
@EduPonz EduPonz added needs-review PR that is ready to be reviewed ci-pending PR which CI is running labels Mar 26, 2024
@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 2, 2024

@EduPonz would you mind addressing the conflicts here ?

@EduPonz
Refs #20592: Fix hidden overloaded virtual methods
a8125c3 
* 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)
@EduPonz EduPonz force-pushed the hotfix/hidden_overloads/2.6.x branch from faa3bb0 to a8125c3 Compare April 3, 2024 07:16
@Mario-DL Mario-DL self-requested a review April 3, 2024 11:37
Mario-DL
Mario-DL approved these changes Apr 3, 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

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 3, 2024

@richiprosima please test this

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 4, 2024

@richiprosima please test mac

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 5, 2024

Mac ci failures not significative, it tool 11 hours, meaning a congested server. Reordering it again

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 5, 2024

@richiprosima please test mac

3 similar comments
@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 8, 2024

@richiprosima please test mac

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 9, 2024

@richiprosima please test mac

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 10, 2024

@richiprosima please test mac

@Mario-DL
Copy link
Copy Markdown
Contributor

Mario-DL commented Apr 10, 2024

Failed tests are unrelated to this PR.
We are not waiting jenkins mac ci this time since it is unstable. Ready to merge

@Mario-DL Mario-DL added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed needs-review PR that is ready to be reviewed ci-pending PR which CI is running labels Apr 10, 2024
@EduPonz EduPonz merged commit dc6aaf3 into 2.6.x Apr 10, 2024
@EduPonz EduPonz deleted the hotfix/hidden_overloads/2.6.x branch April 10, 2024 06:33
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

ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.

Projects

None yet

Milestone

v2.6.8

Development

Successfully merging this pull request may close these issues.

2 participants

@EduPonz @Mario-DL