-
Notifications
You must be signed in to change notification settings - Fork 139
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
[add_launch_test] Correct default python executable for windows debug #239
[add_launch_test] Correct default python executable for windows debug #239
Conversation
Hmm, silently ignoring a user provided argument doesn't sound like a good idea. But I see your point, in which case we might as well remove the argument and remove use cases throughout the codebase. I do wonder why this isn't the case for |
I'm not ignoring the user argument ... it's inside an 'if(NOT _add_launch_test_PYTHON_EXECUTABLE)'. |
Oops, I misread the patch :) Anyways, the question as to why this isn't implemented in |
Just to provide more context about |
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
5102f42
to
5ec8e9b
Compare
I've fixed conflicts with master. I've also fixed another bug 5ec8e9b. This should resolve ros2/build_farmer#200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I believe something similar should be done in I'm looking into it. |
Just what the title says.
@hidmic I think that your original request was doing this in each of the places where
add_launch_test
is used, but I really think this should be the default behavior. What do you think?And maybe, we should apply a similar change to
ament_add_pytest_test
...