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

Simplify code in CDRMessage #3855

Merged
merged 5 commits into from
Nov 8, 2023
Merged

Simplify code in CDRMessage #3855

merged 5 commits into from
Nov 8, 2023

Conversation

paxifaer
Copy link
Contributor

@paxifaer paxifaer commented Sep 16, 2023 •
edited by MiguelCompany
Loading

Description

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • 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.
  • Changes are ABI compatible.
  • 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.
  • 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.

@paxifaer
for test
dd71a75 
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
@paxifaer
1。Removed the macro and replaced it with a template function addPrimi…
4b014af 
…tive to handle the addition of primitive types.

2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
@paxifaer
fix little bug.I put an extra A in a certain line
67fe73b 
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
@paxifaer
change the style
21fa068 
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
@paxifaer
Copy link
Contributor Author

Hello, I submitted a code before because the style did not meet the requirements, so I resubmitted it. But why is this happening now? Expected — Waiting for status to be reported.

@EduPonz
Copy link

EduPonz commented Sep 18, 2023

Hi @paxifaer,

Thanks for the effort you are putting into this. The status is waiting because we need to approve the CI run for external contributions. We normally only do that once we have taken a look at the proposed changes to save resources. At the moment we are quite busy merging the final PRs for the incoming Fast DDS version. Nonetheless we will review this work in the upcoming week and get back to you.

@paxifaer
Copy link
Contributor Author

Thanks for the effort you are putting into this. The status is waiting because we need to approve the CI run for external contributions. We normally only do that once we have taken a look at the proposed changes to save resources. At the moment we are quite busy merging the final PRs for the incoming Fast DDS version. Nonetheless we will review this work in the upcoming week and get back to you.

Hi @paxifaer,

Thanks for the effort you are putting into this. The status is waiting because we need to approve the CI run for external contributions. We normally only do that once we have taken a look at the proposed changes to save resources. At the moment we are quite busy merging the final PRs for the incoming Fast DDS version. Nonetheless we will review this work in the upcoming week and get back to you.

No problem sir. Hahaha, the days when a version is released always make people anxious. You go ahead and release the version first. I'm not in a hurry.

@paxifaer
Copy link
Contributor Author

Anybody review?

@JLBuenoLopez JLBuenoLopez added this to the v2.12.1 milestone Sep 27, 2023
@JLBuenoLopez
Copy link
Contributor

Anybody review?

@paxifaer, right now we are working on releasing Fast DDS v2.12.0. This PR is not considered to be included in this release as it was opened after the internally freeze (it is not publicly announced). I have set the milestone to v2.12.1 so it will be reviewed for the next release for sure. Sorry if the development process is a little frustrating. Be patient and we will reach back to you.

MiguelCompany
MiguelCompany requested changes Oct 18, 2023
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.

Other than my suggestion below, this looks good to me

include/fastdds/rtps/messages/CDRMessage.hpp Outdated Show resolved Hide resolved
@paxifaer
accept MiguelCompany comment
c16378a 
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
@MiguelCompany
Copy link
Member

@richiprosima Please test this

@MiguelCompany MiguelCompany changed the title I think the code implementation of the message encapsulation part is too redundant. Should be simplified some to enhance readability Simplify code in CDRMessage Oct 21, 2023
@paxifaer
Copy link
Contributor Author

@richiprosima Please test this

I tested it on my local Ubuntu 20.04 and it was no problem. Could you please tell me in which environment your cicd service discovery program is run.

@MiguelCompany
Copy link
Member

@paxifaer Thanks for your contribution (and your patience)!

@MiguelCompany MiguelCompany merged commit 7b6b362 into eProsima:master Nov 8, 2023
8 of 14 checks passed
@MiguelCompany
Copy link
Member

@Mergifyio backport 2.11.x 2.10.x 2.6.x

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 8, 2023 •
edited
Loading

backport 2.11.x 2.10.x 2.6.x

✅ Backports have been created

@mergify mergify bot mentioned this pull request Nov 8, 2023
Merged
mergify bot pushed a commit that referenced this pull request Nov 8, 2023
@paxifaer @mergify
Simplify code in CDRMessage (#3855)
c51dfc1 
* for test
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* 1。Removed the macro and replaced it with a template function addPrimitive to handle the addition of primitive types.
2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* fix little bug.I put an extra A in a certain line

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* change the style

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* accept MiguelCompany comment

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

---------

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
(cherry picked from commit 7b6b362)
mergify bot pushed a commit that referenced this pull request Nov 8, 2023
@paxifaer @mergify
Simplify code in CDRMessage (#3855)
30343a6 
* for test
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* 1。Removed the macro and replaced it with a template function addPrimitive to handle the addition of primitive types.
2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* fix little bug.I put an extra A in a certain line

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* change the style

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* accept MiguelCompany comment

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

---------

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
(cherry picked from commit 7b6b362)
This was referenced Nov 8, 2023
Merged
Merged
EduPonz pushed a commit that referenced this pull request Nov 29, 2023
@mergify @paxifaer
Simplify code in CDRMessage (#3855) (#3998)
6068d34 
* for test
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* 1。Removed the macro and replaced it with a template function addPrimitive to handle the addition of primitive types.
2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* fix little bug.I put an extra A in a certain line

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* change the style

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* accept MiguelCompany comment

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

---------

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
(cherry picked from commit 7b6b362)

Co-authored-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
cferreiragonz pushed a commit that referenced this pull request Dec 5, 2023
@paxifaer @cferreiragonz
Simplify code in CDRMessage (#3855)
c429aa7 
* for test
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* 1。Removed the macro and replaced it with a template function addPrimitive to handle the addition of primitive types.
2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* fix little bug.I put an extra A in a certain line

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* change the style

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* accept MiguelCompany comment

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

---------

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
(cherry picked from commit 7b6b362)
EduPonz pushed a commit that referenced this pull request Dec 12, 2023
@mergify @paxifaer
Simplify code in CDRMessage (#3855) (#3997)
d1c6ec1 
* for test
Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* 1。Removed the macro and replaced it with a template function addPrimitive to handle the addition of primitive types.
2.Each specific function (addOctet, addUInt16, etc.) now calls the addPrimitive function.

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* fix little bug.I put an extra A in a certain line

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* change the style

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

* accept MiguelCompany comment

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>

---------

Signed-off-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
(cherry picked from commit 7b6b362)

Co-authored-by: Parsifal <51481718+paxifaer@users.noreply.github.com>
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
None yet
Projects
None yet
Milestone
v2.12.1
Development

Successfully merging this pull request may close these issues.
4 participants
@paxifaer @EduPonz @JLBuenoLopez @MiguelCompany