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.

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

Fix statistics header file inclusion [11614] #1983

Merged
merged 2 commits into from
May 24, 2021
Merged

Fix statistics header file inclusion [11614] #1983

merged 2 commits into from
May 24, 2021

Conversation

richiware
Copy link
Member

@richiware richiware commented May 21, 2021

Depending on the order of header file inclusion could occur a linkage error. This is because StatisticsBase.hpp always includes StatiticsCommon.hpp, with statistics enable or not. This PR moves content of StatisticsCommonEmpty.hpp into StatisticsCommon.hpp and uses the FASTDDS_STATISTICS define to select implementation.

@richiware richiware changed the title Fix statistics header file inclusion Fix statistics header file inclusion [11614] May 21, 2021
Copy link
Contributor

@MiguelBarro MiguelBarro left a comment

Choose a reason for hiding this comment

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

LGTM and to Github actions
Please note the comment

include/fastdds/statistics/rtps/StatisticsCommon.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@MiguelBarro MiguelBarro left a comment

Choose a reason for hiding this comment

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

LGTM

@richiware
Copy link
Member Author

Failed tests not related. Merging...

@richiware richiware merged commit 7e679b0 into master May 24, 2021
@richiware richiware deleted the bugfix/11612 branch May 24, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants