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

Adds wait_for_node #930

Merged
merged 7 commits into from
Aug 10, 2022
Merged

Adds wait_for_node #930

merged 7 commits into from
Aug 10, 2022

Conversation

gonzodepedro
Copy link
Contributor

Related #823

Signed-off-by: Gonzalo de Pedro [email protected]

Signed-off-by: Gonzalo de Pedro <[email protected]>
@gonzodepedro gonzodepedro self-assigned this Apr 27, 2022
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM, I've left a bunch of minor comments below.

rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/node.py Show resolved Hide resolved
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/node.py Show resolved Hide resolved
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
Signed-off-by: Gonzalo de Pedro <[email protected]>
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM after the test is fixed.

rclpy/test/test_node.py Outdated Show resolved Hide resolved
Signed-off-by: Gonzalo de Pedro <[email protected]>
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

rclpy/rclpy/node.py Outdated Show resolved Hide resolved
@ivanpauno
Copy link
Member

@jacobperron are you planning to pick this up from Gonzo?
If not, I can address the remaining comments.

@jacobperron jacobperron requested a review from ivanpauno June 23, 2022 10:18
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

I have some minor suggestions.
LGTM!

rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/test/test_node.py Outdated Show resolved Hide resolved
@audrow audrow changed the base branch from master to rolling June 28, 2022 14:21
@ivanpauno
Copy link
Member

@jacobperron friendly ping

Signed-off-by: Jacob Perron <[email protected]>
missed these in the previous refactor

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron
Copy link
Member

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@jacobperron jacobperron merged commit 9a5d55e into rolling Aug 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the gonzodepedro/add_wait_for_node branch August 10, 2022 15:55
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