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

Polish the ROS 1 -> 2 CMake instructions #4767

Merged
merged 17 commits into from
Sep 30, 2024
Merged

Polish the ROS 1 -> 2 CMake instructions #4767

merged 17 commits into from
Sep 30, 2024

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Sep 22, 2024 •
edited
Loading

This updates the CMake portion of the ROS 1 to ROS 2 guide, and fixes one bug in the C++ code

  • Included links to real examples in the core (Mostly tf2)
  • Updated section headings to CMakeLists.txt steps appear in the ToC
  • Used modern CMake targets instead of standard CMake variables
  • Used Active voice and second person perspective
  • Removed unnecessary code from example code in Unit tests section
  • Make the example package.xml start with format 2 since that's different from migrating from ROS 1 to ROS 2.

C++ bug fix: use ->publish( instead of .publish(

I have tried out all steps of the example part of the guide to make sure they work.

@sloretz
Polish the ROS 1 -> 2 CMake instructions
8db4dc2 
WIP. Progress so far:

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
@sloretz sloretz self-assigned this Sep 22, 2024
@sloretz
Consistent headings
02148be 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Updates down to Testing section
c9b10bb 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Active voice in Linters section
8f68e76 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Proofreading
c2f5d9e 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
remove none
f08324d 
Signed-off-by: Shane Loretz <[email protected]>
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 26, 2024 •
edited
Loading

HTML artifacts: https://github.com/ros2/ros2_documentation/actions/runs/11110850644/artifacts/1996752065.

To view the resulting site:

  1. Click on the above link to download the artifacts archive
  2. Extract it
  3. Open html-artifacts-4767/index.html in your favorite browser

@sloretz
Misc updates to lower section
fefc16f 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Build system first, assume already using package format 2
1e4abef 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Update instructions, removing unnecessary stuff
ed52172 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Some updates to code changes section
30582c7 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Undo non-CMake changes to save for another PR
50e863b 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Only change CMake code this time
c31a35e 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
more changes
81944b0 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Misc fixes
2f4c020 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Misc fixes after running through example
521ed5d 
Signed-off-by: Shane Loretz <[email protected]>
@sloretz sloretz marked this pull request as ready for review September 29, 2024 23:39
@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Sep 30, 2024
clalancette
clalancette requested changes Sep 30, 2024
View reviewed changes
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.

I've got two pretty minor changes that I think we should make, but this otherwise is a great update.

source/How-To-Guides/Migrating-from-ROS1/Migrating-CPP-Packages.rst Show resolved Hide resolved
source/How-To-Guides/Migrating-from-ROS1/Migrating-CPP-Packages.rst Outdated Show resolved Hide resolved
sloretz and others added 2 commits September 30, 2024 10:03
@sloretz @clalancette
Clearly link ROS 2 style guide
92246d4 
Co-authored-by: Chris Lalancette <[email protected]>
Signed-off-by: Shane Loretz <[email protected]>
@sloretz
Add member_of_group tag code block
53dc7e6 
Signed-off-by: Shane Loretz <[email protected]>
clalancette
clalancette approved these changes Sep 30, 2024
View reviewed changes
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 is a fantastic improvement to these docs. Thanks so much for spending the time on this @sloretz

@clalancette clalancette merged commit 9d6552c into rolling Sep 30, 2024
4 checks passed
@clalancette clalancette deleted the sloretz__polish_ros1_migration_cpp branch September 30, 2024 17:50
mergify bot pushed a commit that referenced this pull request Sep 30, 2024
@sloretz @mergify
Polish the ROS 1 -> 2 CMake instructions (#4767)
afe9b68 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)
@mergify mergify bot mentioned this pull request Sep 30, 2024
Merged
mergify bot pushed a commit that referenced this pull request Sep 30, 2024
@sloretz @mergify
Polish the ROS 1 -> 2 CMake instructions (#4767)
799cdaa 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)

# Conflicts:
#	source/How-To-Guides/Migrating-from-ROS1/Migrating-CPP-Packages.rst
mergify bot pushed a commit that referenced this pull request Sep 30, 2024
@sloretz @mergify
Polish the ROS 1 -> 2 CMake instructions (#4767)
b2c368b 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)

# Conflicts:
#	source/How-To-Guides/Migrating-from-ROS1/Migrating-CPP-Packages.rst
This was referenced Sep 30, 2024
Merged
Merged
clalancette pushed a commit that referenced this pull request Sep 30, 2024
@sloretz @clalancette
Polish the ROS 1 -> 2 CMake instructions (#4767)
d1b596b 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)
clalancette pushed a commit that referenced this pull request Sep 30, 2024
@sloretz @clalancette
Polish the ROS 1 -> 2 CMake instructions (#4767)
79c7e08 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)
clalancette pushed a commit that referenced this pull request Sep 30, 2024
@mergify @sloretz
Polish the ROS 1 -> 2 CMake instructions (#4767) (#4775)
032aa76 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)

Co-authored-by: Shane Loretz <[email protected]>
clalancette pushed a commit that referenced this pull request Sep 30, 2024
@mergify @sloretz
Polish the ROS 1 -> 2 CMake instructions (#4767) (#4776)
d5641e2 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)

Co-authored-by: Shane Loretz <[email protected]>
clalancette pushed a commit that referenced this pull request Sep 30, 2024
@mergify @sloretz
Polish the ROS 1 -> 2 CMake instructions (#4767) (#4777)
23c0cbf 
* Polish the ROS 1 -> 2 CMake instructions

* Use section headings to CMakeLists.txt steps appear in the TOC
* Use modern CMake targets instead of standard CMake variables
* Use Active voice and second person perspective
* Include links to real examples

Signed-off-by: Shane Loretz <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
(cherry picked from commit 9d6552c)

Co-authored-by: Shane Loretz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@ahcorde ahcorde ahcorde approved these changes

@audrow audrow Awaiting requested review from audrow audrow is a code owner

Assignees

@sloretz sloretz

Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sloretz @clalancette @ahcorde