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

Set APPEND_PROJECT_NAME_TO_INCLUDEDIR to ON #21

Merged
merged 1 commit into from
Mar 21, 2022

Conversation

sloretz
Copy link

@sloretz sloretz commented Mar 21, 2022

Part of ros2/ros2#1150 and follow up from eProsima/Fast-DDS#2535

@clalancette
Copy link
Contributor

I have what is maybe a dumb question. Should we be patching this branch directly, or should we maybe be adding a new patch to the https://github.com/ros2-gbp/fastrtps-release/tree/patches/debian/rolling/fastrtps branch? I see "SECURITY=ON" in both branches, so it is not clear to me which way to go. Maybe @cottsay or @nuclearsandwich can clarify here?

@cottsay
Copy link
Contributor

cottsay commented Mar 21, 2022

Should we be patching this branch directly, or should we maybe be adding a new patch to the patches/debian/rolling/fastrtps branch?

tl;dr - never change the patches/* branches directly.

Bloom manages that branch with git-bloom-patch. Always apply the changes to the branch you want patched.

Example documentation: http://wiki.ros.org/bloom/Tutorials/ReleaseThirdParty#Adding_an_Install_Rule_as_a_Patch

@clalancette
Copy link
Contributor

tl;dr - never change the patches/* branches directly.

Thanks! That is exactly the information I was looking for.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks.

@sloretz sloretz merged commit 2e8ad70 into debian/rolling/fastrtps Mar 21, 2022
@sloretz sloretz deleted the sloretz__fastrtps__unique_include branch March 21, 2022 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants