fix add rosdep_update_options #684 #890
Conversation
| @@ -150,6 +150,7 @@ if pull_request: | |||
| ' --ros-version ' + str(ros_version) + | |||
| (' --run-abichecker' if run_abichecker else '') + | |||
| (' --require-gpu-support' if require_gpu_support else '') + | |||
| (' --custom-rosdep-update-options="' + ' '.join(custom_rosdep_update_options) + '"' if custom_rosdep_update_options else '') + | |||
There was a problem hiding this comment.
We should consider changing the argument type for this to use nargs=argparse.REMAINDER to avoid the need to wrap the value in quotes. The approach here is fragile and will break if the arguments need to contain quotes or other characters meaningful to the shell.
I'm not entirely sure if we should block this PR on that, and I'm not sure if it might be a breaking change to do so, but as someone who is clearly utilizing this feature already, I'm hoping you can tell me!
There was a problem hiding this comment.
I use this feature on https://github.com/ros-perception/openslam_gmapping/blob/melodic-devel/.travis.yml#L29, but do not know other people who utilizing this. may be @ipa-mdl is interested in ros-industrial/industrial_ci#579, but does not seem to actually using this.
PR #674 did not work as expected, as reported on ros-industrial/industrial_ci#579
now,
works