Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Add a non verbose (quiet) mode for rosnode info#809

Merged
dirk-thomas merged 1 commit intoros:kinetic-develfrom
spaghetti-:kinetic-devel
May 23, 2016
Merged

Add a non verbose (quiet) mode for rosnode info#809
dirk-thomas merged 1 commit intoros:kinetic-develfrom
spaghetti-:kinetic-devel

Conversation

@spaghetti-
Copy link
Copy Markdown

Addresses #804

Default behavior not changed to avoid regression.

@spaghetti-
Copy link
Copy Markdown
Author

@dirk-thomas This file contains a lot of trailing whitespace which my editor removed by default, but I figured I'd open another pull request for that it makes reading this particular diff pretty annoying. Do let me know if you're interested.

spaghetti-@0d51e75

return buff

def rosnode_info(node_name):
def rosnode_info(node_name, non_verbose):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please avoid inverted logic. Either call the argument quiet or verbose.

Adding a positional argument breaks existing code. Please add the new argument as an optional keyword argument. And the default value should not change the current behavior.

@dirk-thomas
Copy link
Copy Markdown
Member

Thanks for avoiding any whitespace changes. The are usually not being considered for merge anyway since it makes backporting changes between branches unnecessary cumbersome.

@spaghetti-
Copy link
Copy Markdown
Author

thanks for the comments - fixed.

@dirk-thomas
Copy link
Copy Markdown
Member

Thanks for the patch and update.

@dirk-thomas dirk-thomas merged commit 82024ac into ros:kinetic-devel May 23, 2016
@spaghetti- spaghetti- deleted the kinetic-devel branch May 23, 2016 18:05
@spaghetti- spaghetti- restored the kinetic-devel branch May 24, 2016 07:51
rsinnet pushed a commit to MisoRobotics/ros_comm that referenced this pull request Jun 19, 2017
Add a non verbose (quiet) mode for rosnode info
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants