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

Update Understanding-ROS2-Topics.rst #4803

Merged
merged 5 commits into from
Oct 15, 2024
Merged

Update Understanding-ROS2-Topics.rst #4803

merged 5 commits into from
Oct 15, 2024

Conversation

mnhaqq
Copy link
Contributor

@mnhaqq mnhaqq commented Oct 3, 2024

Introduce bw and find commands to understanding topics tutorial

@mnhaqq
Update Understanding-ROS2-Topics.rst
584cff1 
Introduce bw and find commands to understanding topics tutorial

Signed-off-by: mnhaqq <[email protected]>
fujitatomoya
fujitatomoya reviewed Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

thanks for the PR. I think that is okay to add those sub-commands in this section as documentation.

source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
@mnhaqq
Update Understanding-ROS2-Topics.rst
221097b 
Rephrase description of bw command for clarity

Fixes ros2#4610

Signed-off-by: mnhaqq <[email protected]>
@mnhaqq
Copy link
Contributor Author

mnhaqq commented Oct 7, 2024

@fujitatomoya could you please review the updated changes

@mnhaqq
Update Understanding-ROS2-Topics.rst
b5def0e 
Remove redundant lines

Signed-off-by: mnhaqq <[email protected]>
fujitatomoya
fujitatomoya approved these changes Oct 7, 2024
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@fujitatomoya
Copy link
Collaborator

@clalancette @ahcorde either of you can also take a look at this?

@fujitatomoya
Copy link
Collaborator

@mnhaqq github workflow is failing with https://github.com/ros2/ros2_documentation/actions/runs/11225480787/job/31204930999?pr=4803 , can you fix it?

@mnhaqq
Copy link
Contributor Author

mnhaqq commented Oct 8, 2024

@fujitatomoya I've fixed it

clalancette
clalancette requested changes Oct 11, 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.

Overall, this looks good to me. I've suggested a couple of small fixes, then this can go in. Thanks for the contribution!

source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
source/Tutorials/Beginner-CLI-Tools/Understanding-ROS2-Topics/Understanding-ROS2-Topics.rst Outdated Show resolved Hide resolved
@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Oct 11, 2024
@mnhaqq
Update Understanding-ROS2-Topics.rst
068de39 
Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
@mnhaqq
Copy link
Contributor Author

mnhaqq commented Oct 11, 2024

@clalancette requested changes have been made

clalancette
clalancette approved these changes Oct 15, 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.

Thanks for the fixes! This looks good to me.

@clalancette clalancette merged commit a839c3b into ros2:rolling Oct 15, 2024
4 checks passed
mergify bot pushed a commit that referenced this pull request Oct 15, 2024
@mnhaqq @mergify
Update Understanding-ROS2-Topics.rst (#4803)
b181310 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)
@mergify mergify bot mentioned this pull request Oct 15, 2024
Merged
mergify bot pushed a commit that referenced this pull request Oct 15, 2024
@mnhaqq @mergify
Update Understanding-ROS2-Topics.rst (#4803)
09e5a90 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)
mergify bot pushed a commit that referenced this pull request Oct 15, 2024
@mnhaqq @mergify
Update Understanding-ROS2-Topics.rst (#4803)
78bb1ce 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)
This was referenced Oct 15, 2024
Merged
Merged
clalancette pushed a commit that referenced this pull request Oct 15, 2024
@mergify @mnhaqq
Update Understanding-ROS2-Topics.rst (#4803) (#4820)
7a6f424 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)

Co-authored-by: mnhaqq <[email protected]>
clalancette pushed a commit that referenced this pull request Oct 15, 2024
@mergify @mnhaqq
Update Understanding-ROS2-Topics.rst (#4803) (#4819)
a13897b 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)

Co-authored-by: mnhaqq <[email protected]>
clalancette pushed a commit that referenced this pull request Oct 15, 2024
@mergify @mnhaqq
Update Understanding-ROS2-Topics.rst (#4803) (#4818)
8f27b46 
* Update Understanding-ROS2-Topics.rst

Introduce bw and find commands to understanding topics tutorial

* Update Understanding-ROS2-Topics.rst

Rephrase description of bw command for clarity

Fixes #4610

* Update Understanding-ROS2-Topics.rst

Remove redundant lines

* Update Understanding-ROS2-Topics.rst

* Update Understanding-ROS2-Topics.rst

Changes for readability and remove unnecessary lines

Signed-off-by: mnhaqq <[email protected]>
(cherry picked from commit a839c3b)

Co-authored-by: mnhaqq <[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

@fujitatomoya fujitatomoya fujitatomoya approved these changes

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

Assignees
No one assigned
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
@mnhaqq @fujitatomoya @clalancette