Skip to content

print command to stderr if running it fails#22

Merged
dirk-thomas merged 1 commit intomasterfrom
generator_expression
Feb 9, 2016
Merged

print command to stderr if running it fails#22
dirk-thomas merged 1 commit intomasterfrom
generator_expression

Conversation

@dirk-thomas
Copy link
Copy Markdown
Member

This help in debugging when the command in launch file is not invokable.

Since the _TaskException is only used internally and not exposed anywhere I prefixed the name with underscore. Hope that makes sense.

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Feb 5, 2016
@dirk-thomas dirk-thomas self-assigned this Feb 5, 2016
@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Feb 6, 2016

The _ seems ok to me. Though a doc block comment saying that this is only internal would be even less ambiguous.

@dirk-thomas
Copy link
Copy Markdown
Member Author

PEP8 clearly define the semantic of a leading underscore (https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces). The docblock would be redundant - therefore I will leave it without.

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Feb 6, 2016

The leading underscore is typically used to indicate that it's for internal use. But according to the pep: "_single_leading_underscore : weak "internal use" indicator. E.g. from M import * does not import objects whose name starts with an underscore." Adding a doc block to any piece of code is rarely redundant. If it's worth a comment in the PR, it's probably worth a comment in the code, since someone reading the code at a later date will not likely find the comment in the PR.

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Feb 9, 2016

@tfoote does this look ok otherwise (it seems you guys have agreed to disagree on the docstring note)?

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Feb 9, 2016

Yes, otherwise it looks good.

dirk-thomas added a commit that referenced this pull request Feb 9, 2016
print command to stderr if running it fails
@dirk-thomas dirk-thomas merged commit d73bbfd into master Feb 9, 2016
@dirk-thomas dirk-thomas deleted the generator_expression branch February 9, 2016 23:41
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Feb 9, 2016
wjwwood pushed a commit that referenced this pull request Mar 20, 2019
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.

3 participants