Skip to content

[21129] Migrate fastrtps namespace to fastdds - #4898

Merged
EduPonz merged 18 commits into
masterfrom
feature/migrate_fastrtps_namespace
Jun 18, 2024
Merged

[21129] Migrate fastrtps namespace to fastdds#4898
EduPonz merged 18 commits into
masterfrom
feature/migrate_fastrtps_namespace

Conversation

@elianalf

@elianalf elianalf commented Jun 5, 2024

Copy link
Copy Markdown
Contributor

Description

This PR migrates all the fastrtps namespace to fastdds. It also removes all the unnecessary nested namespace and using namespace.

Related PR:
Fast DDS Python: eProsima/Fast-DDS-python#135
Shapes Demo: eProsima/ShapesDemo#141
Fast DDS GEN: eProsima/Fast-DDS-Gen#350
Discovery Server: eProsima/Discovery-Server#87

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.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • ❌ Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • ❌ 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.
    Related documentation PR: [21129] Migrate fastrtps namespace Fast-DDS-docs#813
  • 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.

@elianalf elianalf added this to the v3.0.0 milestone Jun 5, 2024
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 7f89bd2 to 8333472 Compare June 10, 2024 08:35
@elianalf
elianalf marked this pull request as ready for review June 10, 2024 09:53
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 5590928 to 19234a4 Compare June 10, 2024 10:31
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 19234a4 to 3a0ce6d Compare June 12, 2024 06:32
@elianalf
elianalf requested a review from EduPonz June 12, 2024 06:36
@github-actions github-actions Bot added the ci-pending PR which CI is running label Jun 12, 2024
@elianalf
elianalf requested review from EduPonz and removed request for EduPonz June 12, 2024 06:45
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 3e559b4 to 0053c97 Compare June 12, 2024 09:26
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch 3 times, most recently from 86de509 to a14d4b6 Compare June 14, 2024 09:43

@EduPonz EduPonz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. StatisticsCommon.hpp has a fastrtps reference
  2. StatisticsBase.hpp has a fastrtps reference
  3. utils includes have a guard with FASTRTPS
  4. We can remove the XFAIL_FASTRTPS_TESTS in xfail_tests.cmake
  5. Remove fastdds:: from all headers in include. It is not needed
  6. Remove fastdds:: from all src files. It is not needed almost anywhere
  7. Almost all using fastdds:: that are not using fastdds::rtps are not needed anymore. Same happens with using eprosima::fastdds:: outside of the examples

Comment thread include/fastdds/dds/domain/DomainParticipant.hpp
Comment thread include/fastdds/dds/core/condition/WaitSet.hpp Outdated
Comment thread include/fastdds/dds/core/policy/ParameterTypes.hpp Outdated
Comment thread src/cpp/fastdds/publisher/PublisherImpl.cpp Outdated

@EduPonz EduPonz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm afraid the following are not yet addressed in full:

  1. Remove fastdds:: from all headers in include. It is not needed
  2. Remove fastdds:: from all src files. It is not needed almost anywhere
  3. Almost all using fastdds:: that are not using fastdds::rtps are not needed anymore.
  4. Same happens with using eprosima::fastdds:: outside of the examples

@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 8aa916e to a151857 Compare June 17, 2024 06:56
elianalf added 8 commits June 17, 2024 08:57
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
elianalf added 7 commits June 17, 2024 08:59
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from a151857 to 4b8c06d Compare June 17, 2024 07:01
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
elianalf force-pushed the feature/migrate_fastrtps_namespace branch from 4b8c06d to 28ba6ca Compare June 17, 2024 07:23
@EduPonz
EduPonz self-requested a review June 17, 2024 09:36
EduPonz
EduPonz previously approved these changes Jun 17, 2024

@EduPonz EduPonz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This LGTM with green CI

Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
@elianalf
elianalf requested review from EduPonz and removed request for EduPonz June 17, 2024 12:50
@EduPonz

EduPonz commented Jun 17, 2024

Copy link
Copy Markdown

@richiprosima please test_3 discovery-server

@EduPonz
EduPonz merged commit 332db84 into master Jun 18, 2024
@EduPonz
EduPonz deleted the feature/migrate_fastrtps_namespace branch June 18, 2024 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-pending PR which CI is running first-to-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants