Skip to content

Conversation

@BMarchi
Copy link
Contributor

@BMarchi BMarchi commented Oct 30, 2019

For parameters without a "." prefix, the code wasn't working as it should. Connected to ros2/ros2cli#389

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM, can we add a test or do a sanity linter check at least?

@BMarchi
Copy link
Contributor Author

BMarchi commented Nov 12, 2019

What do you mean by sanity linter?

@hidmic
Copy link
Contributor

hidmic commented Nov 12, 2019

What do you mean by sanity linter?

Even if no tests are available, do a CI run on e.g. Linux to make sure at least linters are passing.

@mjcarroll
Copy link
Member

The Epr job runs linters.

@hidmic
Copy link
Contributor

hidmic commented Nov 12, 2019

Oh, that's right. Then go ahead if the ROSBoss approves.

@mjcarroll mjcarroll merged commit c78b02b into master Nov 12, 2019
@delete-merged-branch delete-merged-branch bot deleted the BMarchi/add_param_cli_tests branch November 12, 2019 20:45
jaisontj pushed a commit to aws-ros-dev/rclpy that referenced this pull request Nov 19, 2019
suab321321 pushed a commit to suab321321/rclpy that referenced this pull request Jan 31, 2020
Signed-off-by: Brian Ezequiel Marchi <[email protected]>
Signed-off-by: AbhinavSingh <[email protected]>
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