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

[16328] Set FASTDDS_STATISTICS CMake option to ON #3135

Merged
merged 6 commits into from
Dec 12, 2022
Merged

[16328] Set FASTDDS_STATISTICS CMake option to ON #3135

merged 6 commits into from
Dec 12, 2022

Conversation

MRicoIE2CS
Copy link
Contributor

@MRicoIE2CS MRicoIE2CS commented Dec 7, 2022 •
edited by MiguelCompany
Loading

Signed-off-by: Mikel Rico [email protected]

Set FASTDDS_STATISTICS CMake option to ON

Contributor Checklist

  • Commit messages follow the project guidelines.
  • [N/A] 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.
  • [N/A] Any new/modified methods have been properly documented using Doxygen.
  • [N/A] Fast DDS test suite has been run locally.
  • [N/A] Changes are ABI compatible.
  • [N/A] Changes are API compatible.
  • [N/A] Documentation builds and tests pass locally.
  • 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.
    Related documentation PR: eProsima/Fast-DDS-docs# (PR)

Reviewer Checklist

  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@MRicoIE2CS
Refs #16328: versions.md
ca7c345 
Signed-off-by: Mikel Rico <[email protected]>
@MRicoIE2CS
Copy link
Contributor Author

@richiprosima please test this

@EduPonz EduPonz added this to the v2.9.0 milestone Dec 7, 2022
@MiguelCompany
Copy link
Member

@richiprosima Please test this

MiguelCompany
MiguelCompany requested changes Dec 7, 2022
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.

BlackboxTests_DDS_PIM.DDSDiscovery.AddDiscoveryServerToList is failing consistently on all platforms

@EduPonz
Refs #16328: Fix BlackboxTests_DDS_PIM.DDSDiscovery.AddDiscoveryServe…
bcb24f1 
…rToList when compiling with Statistics

Signed-off-by: Eduardo Ponz <[email protected]>
@EduPonz
Copy link

EduPonz commented Dec 7, 2022 •
edited
Loading

BlackboxTests_DDS_PIM.DDSDiscovery.AddDiscoveryServerToList is failing consistently on all platforms

Addressed in bcb24f1.

Because of the statistics properties, the DomainParticipantQos used to create a participant may not the same as the one obtained when calling DomainParticipant::get_qos() right after creation. I think we should probably reflect that in the versions.md and probably have a Fast DDS-Docs PR where this is explained.

@EduPonz
Copy link

EduPonz commented Dec 9, 2022

@richiprosima please test this

@EduPonz EduPonz added ci-pending PR which CI is running doc-pending Issue or PR which is pending to be documented labels Dec 9, 2022
@MRicoIE2CS MRicoIE2CS mentioned this pull request Dec 9, 2022
Merged
@MiguelCompany
Copy link
Member

@richiprosima Please test Windows

@EduPonz
Copy link

EduPonz commented Dec 9, 2022

@richiprosima please test linux

@EduPonz
Copy link

EduPonz commented Dec 10, 2022

@richiprosima please test windows mac

@EduPonz
Copy link

EduPonz commented Dec 10, 2022

@richiprosima please test this

@MiguelCompany MiguelCompany merged commit 5b7cc78 into master Dec 12, 2022
@MiguelCompany MiguelCompany deleted the config/statistics_on branch December 12, 2022 06:59
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 doc-pending Issue or PR which is pending to be documented
Projects
None yet
Milestone
v2.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@MRicoIE2CS @MiguelCompany @EduPonz