Skip to content

Adding namespaces#195

Closed
richiware wants to merge 1 commit intoros2:masterfrom
richiware:fix/namespaces
Closed

Adding namespaces#195
richiware wants to merge 1 commit intoros2:masterfrom
richiware:fix/namespaces

Conversation

@richiware
Copy link
Copy Markdown
Contributor

We removed in Fast-RTPS a using eprosima::fastrtps and a using eprosima::fastrtps::rtps located in a header file. Some users have had problem with this. This PR resolves compilation errors before upload our commit to Github.

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Apr 13, 2018
@mikaelarguedas
Copy link
Copy Markdown
Member

Thanks @richiware for the headsup.

We removed in Fast-RTPS a using eprosima::fastrtps and a using eprosima::fastrtps::rtps located in a header file. Some users have had problem with this. This PR resolves compilation errors before upload our commit to Github.

Is this commit available somewhere on the Fast-RTPS Github repository for us to test?

This PR currently violates our styleguide and will fail the tests. I have a similar set of commit on this branch that I think adress the same problem and I'd like to test against your namespace removal.

@mikaelarguedas mikaelarguedas added more-information-needed Further information is required requires-changes labels Apr 13, 2018
@richiware
Copy link
Copy Markdown
Contributor Author

@mikaelarguedas Three months ago a fellow worker made changes in our internal develop branch to solve a problem with namespaces. This week we decided to merge this changes in our internal master branch. I don't want to upload it to Github before be sure not break ROS2.

Today I tested our internal master branch (with the fixing in namespaces) with ROS2 and rmw_fastrtps didn't compile. I was not aware about your changes and decided to upload a PR that solves the problem. But you can deny it.

I've pushed to Github our internal master branch in a new branch for helping you to test.

@mikaelarguedas mikaelarguedas mentioned this pull request Apr 13, 2018
@mikaelarguedas
Copy link
Copy Markdown
Member

Thanks @richiware for being proactive on this! and for pushing a branch with the changes for us to test 👍. Unfortunately it currently doesnt compile.
I opened #196 with similar changes, I will comment here as soon as #196 is ready to be merged so that you can roll out your changes in Fast-RTPS.

@mikaelarguedas
Copy link
Copy Markdown
Member

@richiware #196 has been merged so rmw_fastrtps_cpp should work with the new namespace changes.

Thanks!

@mikaelarguedas mikaelarguedas removed the in progress Actively being worked on (Kanban column) label Apr 16, 2018
@richiware
Copy link
Copy Markdown
Contributor Author

I've updated our master branch.

@mikaelarguedas
Copy link
Copy Markdown
Member

thanks @richiware for the follow-up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

more-information-needed Further information is required requires-changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants