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

[21352] Refactor TopicDataType #5066

Merged
merged 14 commits into from
Jul 18, 2024
Merged

[21352] Refactor TopicDataType #5066

merged 14 commits into from
Jul 18, 2024

Conversation

richiware
Copy link
Member

@richiware richiware commented Jul 12, 2024 •
edited by MiguelCompany
Loading

Description

Implementation of XCDR introduces new functions in TopicDataType class. But the old one remained. This PR removes them before launching the major version 3, and also performs additional refactorings:

  • Change method names to snake_case and have more meaningful names.
  • Change arguments from pointer to reference where applicable
  • Use std::string instead of char* on name getters and setters

Depends on:

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
  • 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.
  • 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.
  • N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@richiware richiware changed the title Remove old TopicDataType functions [21352] Remove old TopicDataType functions Jul 12, 2024
MiguelCompany
MiguelCompany requested changes Jul 12, 2024
View reviewed changes
include/fastdds/dds/topic/TopicDataType.hpp Outdated Show resolved Hide resolved
include/fastdds/dds/topic/TopicDataType.hpp Outdated Show resolved Hide resolved
src/cpp/fastdds/publisher/DataWriterImpl.cpp Outdated Show resolved Hide resolved
src/cpp/fastdds/publisher/DataWriterImpl.hpp Show resolved Hide resolved
include/fastdds/dds/topic/TopicDataType.hpp Show resolved Hide resolved
@richiware richiware requested review from MiguelCompany and removed request for MiguelCompany July 15, 2024 10:41
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 15, 2024
@richiware richiware requested review from MiguelCompany and removed request for MiguelCompany July 15, 2024 10:44
MiguelCompany
MiguelCompany requested changes Jul 15, 2024
View reviewed changes
include/fastdds/dds/topic/TopicDataType.hpp Outdated Show resolved Hide resolved
include/fastdds/dds/topic/TopicDataType.hpp Outdated Show resolved Hide resolved
include/fastdds/dds/topic/TopicDataType.hpp Outdated Show resolved Hide resolved
src/cpp/fastdds/subscriber/history/DataReaderHistory.cpp Outdated Show resolved Hide resolved
src/cpp/fastdds/xtypes/dynamic_types/DynamicPubSubType.cpp Outdated Show resolved Hide resolved
@MiguelCompany MiguelCompany added this to the v3.0.0 milestone Jul 16, 2024
This was referenced Jul 16, 2024
Merged
Merged
Merged
@richiware richiware mentioned this pull request Jul 17, 2024
Merged
7 tasks
@MiguelCompany MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany July 17, 2024 06:24
@MiguelCompany MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany July 17, 2024 06:35
MiguelCompany
MiguelCompany previously approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

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

MiguelCompany
MiguelCompany previously approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

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

@richiware richiware requested review from MiguelCompany and removed request for MiguelCompany July 17, 2024 09:54
@richiware richiware requested review from MiguelCompany and removed request for MiguelCompany July 17, 2024 14:00
@EduPonz EduPonz mentioned this pull request Jul 17, 2024
Merged
7 tasks
@richiware richiware added skip-ci Automatically pass CI and removed ci-pending PR which CI is running labels Jul 18, 2024
@richiware
Copy link
Member Author

CI passed in 977befe

@richiware
Refs #21349. Remove old functions
d26ab59 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Apply suggestions: Update TopicDataType
eee03dc 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Rename GEN_API_VER
addb202 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Apply suggestions
b267b1c 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Apply suggestions
a54f440 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Using new compute_key
fc4bd1b 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Regenerate code
988e93c 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Apply suggestions
b1ceea7 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Fix bitsets after regenerate code
b819f08 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Fix test
146ea04 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Regenerate code after rebase
eb276b0 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Fix in fastdds_python
b9ee051 
Signed-off-by: Ricardo González Moreno <[email protected]>
@richiware
Refs #21349. Fix doxygen
c04ae29 
Signed-off-by: Ricardo González Moreno <[email protected]>
@MiguelCompany MiguelCompany added no-test Skip CI tests if PR marked with this label and removed skip-ci Automatically pass CI labels Jul 18, 2024
@MiguelCompany MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany July 18, 2024 08:33
@github-actions github-actions bot added the ci-pending PR which CI is running label Jul 18, 2024
@richiware richiware merged commit 50b6494 into master Jul 18, 2024
13 of 14 checks passed
@richiware richiware deleted the feature/21349 branch July 18, 2024 10:39
@JesusPoderoso JesusPoderoso mentioned this pull request Aug 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany approved these changes

Assignees
No one assigned
Labels
ci-pending PR which CI is running no-test Skip CI tests if PR marked with this label
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@richiware @MiguelCompany