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

SROS2 cmake restructure #74

Merged
merged 1 commit into from
Jan 11, 2019
Merged

Conversation

ross-desmond
Copy link
Contributor

Restructures sros2 to the following: @jacobperron

Just a reminder the security_helpers folder in sros2 may want to be split out to another package as it does not build under the current sros2 package structure. If we believe this helper macro should stay in sros2 then I'll need to restructure to build sros2 python modules and the helper macro, thoughts?

IMO, the cmake package can remain in this repo (but remain a separate package) as it matches the repo description. However, I would consider remaining the package to something like sros2_cmake, since its main purpose is providing cmake macros for interacting with ROS 2 security.
I would restructure this repo so that the two packages are at top along with the general docs about sros2:

sros2
  |_ sros2 (python lib)
  |_ test
  |_ package.xml
  |_ setup.py
  |_ ... etc
sros2_cmake
  |_ cmake
    |_ ros2_create_keystore.cmake
    |_ ros2_secure_node.cmake
  |_ CMakeLists.txt
  |_ package.xml
  |_ ... etc
CONTRIBUTING.md
LICENSE
README.md
SROS2_Linux.md
SROS2_MacOS.md
SROS2_Windows.md

I'd like to get some other thoughts on this proposal.

If we go this approach, I think it would be better if the cmake package is added in a separate PR as it is distinct from the CLI changes and is a larger change.

Depends on #71

@tfoote tfoote added the in review Waiting for review (Kanban column) label Jan 11, 2019
@ruffsl
Copy link
Member

ruffsl commented Jan 11, 2019

Could you separate the migration of the sros2 python lib folder rename/restructure vs the addition of the sros2_cmake stuff into seperate PRs? Then I could work on rebasing my xml PR sooner while the cmake stuff is still in review.

Enables addition of new colcon built packages in this repo
Copy link
Member

@ruffsl ruffsl left a comment

Choose a reason for hiding this comment

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

LGTM

@ruffsl ruffsl merged commit 0ace86d into ros2:master Jan 11, 2019
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Jan 14, 2019
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.

4 participants