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 About-Nodes.rst #3917

Merged
merged 4 commits into from
Sep 18, 2023
Merged

Update About-Nodes.rst #3917

merged 4 commits into from
Sep 18, 2023

Conversation

pururval
Copy link
Contributor

There is a discrepancy between the client-server definitions from here and their actual hyperlinks. We may need the same fix in Iron and other versions.

clalancette
clalancette requested changes Sep 14, 2023
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 agree that the current wording is backwards. However, I think the whole thing is unclear. I've suggested some alternative wording; what do you think?

source/Concepts/Basic/About-Nodes.rst Show resolved Hide resolved
source/Concepts/Basic/About-Nodes.rst Outdated Show resolved Hide resolved
clalancette
clalancette requested changes Sep 15, 2023
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.

OK, we are starting to get closer here. I have two more suggestions. Thanks for the iterations so far.

source/Concepts/Basic/About-Nodes.rst Outdated Show resolved Hide resolved
source/Concepts/Basic/About-Nodes.rst Outdated Show resolved Hide resolved
pururval and others added 2 commits September 18, 2023 12:32
@pururval @clalancette
Update source/Concepts/Basic/About-Nodes.rst
fdc849e 
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
@pururval @clalancette
Update source/Concepts/Basic/About-Nodes.rst
4799e2f 
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
clalancette
clalancette approved these changes Sep 18, 2023
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.

Looks good to me, thank you for iterating!

@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Sep 18, 2023
@clalancette clalancette merged commit 4c9c035 into ros2:rolling Sep 18, 2023
3 checks passed
mergify bot pushed a commit that referenced this pull request Sep 18, 2023
@pururval @mergify
Update About-Nodes.rst (#3917)
01f5419 
* Update About-Nodes.rst

Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit 4c9c035)
@mergify mergify bot mentioned this pull request Sep 18, 2023
Merged
mergify bot pushed a commit that referenced this pull request Sep 18, 2023
@pururval @mergify
Update About-Nodes.rst (#3917)
a8d1dad 
* Update About-Nodes.rst

Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit 4c9c035)
@mergify mergify bot mentioned this pull request Sep 18, 2023
Merged
clalancette pushed a commit that referenced this pull request Sep 18, 2023
@mergify @pururval
Update About-Nodes.rst (#3917) (#3924)
847c4e4 
* Update About-Nodes.rst

Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit 4c9c035)

Co-authored-by: Pururva Lakkad <pururval@buffalo.edu>
clalancette pushed a commit that referenced this pull request Sep 18, 2023
@mergify @pururval
Update About-Nodes.rst (#3917) (#3925)
5d45cf9 
* Update About-Nodes.rst

Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit 4c9c035)

Co-authored-by: Pururva Lakkad <pururval@buffalo.edu>
@pururval pururval deleted the patch-1 branch September 19, 2023 17:17
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

@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.
2 participants
@pururval @clalancette