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

Fix/param namespace association 894 #1132

Merged
merged 2 commits into from Jun 15, 2023
Merged

Fix/param namespace association 894 #1132

merged 2 commits into from Jun 15, 2023

Conversation

devrite
Copy link
Contributor

@devrite devrite commented Jun 7, 2023

Fixes #894

fujitatomoya
fujitatomoya approved these changes Jun 7, 2023
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, @devrite thanks for the contribution.

@jacobperron can you do another review?

clalancette
clalancette requested changes Jun 14, 2023
View reviewed changes
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
@devrite devrite force-pushed the fix/param_namespace_association_894 branch from 3d8e6ac to 02cfd5c Compare June 14, 2023 15:02
clalancette
clalancette approved these changes Jun 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.

This looks good to me, thanks for iterating. I'll run CI on it next.

@clalancette
Copy link
Contributor

clalancette commented Jun 14, 2023 •
edited

CI:

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

@clalancette
Copy link
Contributor

@devrite Due to some unrelated changes, this needs to be rebased onto the rolling branch to pass CI. Let me know once you've done that and I'll kick off another round.

@devrite
Add regression test for ros2#894
400d289 
Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
@devrite
Fix ros2#894
e7356aa 
Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
@devrite devrite force-pushed the fix/param_namespace_association_894 branch from 02cfd5c to e7356aa Compare June 14, 2023 20:36
@devrite
Copy link
Contributor Author

devrite commented Jun 14, 2023 •
edited

@clalancette Cleared for launch!

@clalancette
Copy link
Contributor

clalancette commented Jun 15, 2023 •
edited

Thanks, here we go:

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

@clalancette clalancette merged commit 309f755 into ros2:rolling Jun 15, 2023
3 checks passed
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

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

@adityapande-1995 adityapande-1995 Awaiting requested review from adityapande-1995 adityapande-1995 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot load parameters declared with namespace from yaml or command line
3 participants
@devrite @clalancette @fujitatomoya