Skip to content

Make transport public headers private [10002]#1577

Merged
MiguelCompany merged 30 commits intomasterfrom
fix/remove_public_headers_transport_layer
Feb 26, 2021
Merged

Make transport public headers private [10002]#1577
MiguelCompany merged 30 commits intomasterfrom
fix/remove_public_headers_transport_layer

Conversation

@JLBuenoLopez
Copy link
Copy Markdown
Contributor

@JLBuenoLopez JLBuenoLopez commented Nov 23, 2020

No description provided.

@JLBuenoLopez JLBuenoLopez added the skip-ci Automatically pass CI label Nov 23, 2020
@JLBuenoLopez JLBuenoLopez changed the title Make transport public headers private Make transport public headers private [10002] Nov 23, 2020
MiguelCompany
MiguelCompany requested changes Nov 23, 2020
View reviewed changes
Copy link
Copy Markdown
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.

I think we should take the chance to clean-up all the headers under include/fastdds/rtps/transport/

Start by doing an uncrustify over all of them

@JLBuenoLopez
Copy link
Copy Markdown
Contributor Author

JLBuenoLopez commented Nov 23, 2020 •
edited
Loading

Start by doing an uncrustify over all of them

@MiguelCompany,

Done in dabe29f

MiguelCompany
MiguelCompany requested changes Nov 23, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Partial review

MiguelCompany
MiguelCompany requested changes Nov 23, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Partial review 2: all public headers reviewed

MiguelCompany
MiguelCompany requested changes Nov 24, 2020
View reviewed changes
@rsanchez15 rsanchez15 force-pushed the feature/increase-coverage branch from 2fb6834 to 80b27f1 Compare November 27, 2020 08:19
@JLBuenoLopez JLBuenoLopez force-pushed the fix/remove_public_headers_transport_layer branch 2 times, most recently from 8175c55 to 4d7eeff Compare December 4, 2020 11:19
MiguelCompany
MiguelCompany requested changes Dec 4, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Copy assignment on public headers need RTPS_DllAPI

Base automatically changed from feature/increase-coverage to master December 9, 2020 11:11
@imontesino imontesino force-pushed the fix/remove_public_headers_transport_layer branch 2 times, most recently from 70e5157 to 80b7478 Compare December 23, 2020 12:40
@imontesino imontesino removed the skip-ci Automatically pass CI label Dec 23, 2020
@richiware
Copy link
Copy Markdown
Member

richiware commented Dec 23, 2020

Build status:

  • Linux Build Status
  • Linux aarch64Build Status
  • Mac Build Status
  • Windows Build Status

MiguelCompany
MiguelCompany requested changes Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Apart from my comments below, a blackbox test checking copy constructors and assignment operators of the transport descriptors would be nice

@JLBuenoLopez JLBuenoLopez marked this pull request as draft January 13, 2021 08:20
@JLBuenoLopez JLBuenoLopez added the skip-ci Automatically pass CI label Jan 13, 2021
@JLBuenoLopez
Copy link
Copy Markdown
Contributor Author

JLBuenoLopez commented Jan 13, 2021

Converted to draft and labeled skip-ci until introducing the Blackbox tests asked by the reviewer.

@jparisu jparisu removed the skip-ci Automatically pass CI label Jan 20, 2021
@jparisu jparisu marked this pull request as ready for review January 20, 2021 11:48
@richiware
Copy link
Copy Markdown
Member

richiware commented Jan 20, 2021

Build status:

  • Linux Build Status
  • Linux aarch64Build Status
  • Mac Build Status
  • Windows Build Status

MiguelCompany
MiguelCompany requested changes Feb 1, 2021
View reviewed changes
@jparisu jparisu added the no-test Skip CI tests if PR marked with this label label Feb 2, 2021
MiguelCompany
MiguelCompany requested changes Feb 2, 2021
View reviewed changes
Copy link
Copy Markdown
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.

  1. I would name the macro FASTDDS_TODO_BEFORE
  2. Fix the reported linter issues

MiguelCompany
MiguelCompany previously approved these changes Feb 3, 2021
View reviewed changes
@eProsima eProsima deleted a comment from richiware Feb 3, 2021
@richiware
Copy link
Copy Markdown
Member

richiware commented Feb 4, 2021

Build status:

  • Linux Build Status
  • Linux aarch64Build Status
  • Mac Build Status
  • Windows Build Status

Ignacio Montesino and others added 14 commits February 4, 2021 12:52
@MiguelCompany
rerun CI
e8f6ed5 
Signed-off-by: Ignacio Montesino <ignaciomontesino@eprosima.com>
@JLBuenoLopez @MiguelCompany
Refs #9957: applied review suggestions
3fb1406 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez @MiguelCompany
Refs #9957: SHM Blackbox test
0643d4b 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez @MiguelCompany
Refs #9957: TCPv4 Blackbox test (preliminar)
6728e33 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez @MiguelCompany
Refs #9957: add comparison operator to TransportDescriptors
4ffc205 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez @MiguelCompany
Refs #9957: use comparison operator in SHM test
eb2752d 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@MiguelCompany
Refs #9957: fix == operator in TCPv4Transport
0ec7cba 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: add test for copy and == operator for TransportDescriptor…
6320051 
… classes

Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: uncrustify
3eb625a 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: uncrustify bis
756f50d 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: new TODO macro and fix typo
8ffcb7a 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: apply changes
9dd3e6d 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs #9957: uncrustify
799d7a2 
Signed-off-by: jparisu <javierparis@eprosima.com>
@MiguelCompany
Refs 10002. Fixed DDSBlackboxTestsDataSharing.
0ddc9c7 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany force-pushed the fix/remove_public_headers_transport_layer branch from b20472a to 0ddc9c7 Compare February 4, 2021 12:07
@richiware
Copy link
Copy Markdown
Member

richiware commented Feb 4, 2021

Build status:

  • Linux Build Status
  • Linux aarch64Build Status
  • Mac Build Status
  • Windows Build Status

MiguelCompany
MiguelCompany previously approved these changes Feb 9, 2021
View reviewed changes
Copy link
Copy Markdown
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

@MiguelCompany MiguelCompany added the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Feb 9, 2021
@MiguelCompany MiguelCompany added this to the v2.3.0 milestone Feb 9, 2021
@MiguelCompany MiguelCompany mentioned this pull request Feb 19, 2021
Merged
@EduPonz EduPonz removed the no-test Skip CI tests if PR marked with this label label Feb 25, 2021
@EduPonz
Copy link
Copy Markdown

EduPonz commented Feb 25, 2021

@richiprosima Please test this

@MiguelCompany
Copy link
Copy Markdown
Member

MiguelCompany commented Feb 25, 2021

@richiprosima Please test mac

@MiguelCompany
Copy link
Copy Markdown
Member

MiguelCompany commented Feb 25, 2021

@richiprosima Please test this

@MiguelCompany
Refs 10002. Fix BalckboxTestsTransportUDP.
fc7eab7 
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany merged commit bdbd72c into master Feb 26, 2021
@MiguelCompany MiguelCompany deleted the fix/remove_public_headers_transport_layer branch February 26, 2021 06:15
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

+1 more reviewer

@jparisu jparisu jparisu left review comments

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

Development

Successfully merging this pull request may close these issues.

6 participants

@JLBuenoLopez @richiware @EduPonz @MiguelCompany @jparisu @imontesino