search

ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
124 stars 139 forks source link

Add option to set output_format via environment variable #713

Open SammyRamone opened 1 year ago

SammyRamone commented 1 year ago

This PR would close https://github.com/ros2/launch/issues/664

I used a constant to get the default value instead of self.__init__.__defaults__[6] since adding any further arguments would break the code in that case. Any feedback is welcome.

SammyRamone commented 1 year ago

Sorry for the delay, I was on a short vaccation. As stated in my reply above, I changed the implementation so that the envvar overrides the everything. I also added a small note in the doc string.

wjwwood commented 1 year ago

@clalancette's proposal is fine with me.

If we wanted to be really thorough, we could have what you suggest as well as an "override" env var which let you explicitly set it even if something else has been set in the code. This would be just for debugging situations I guess. But doing just what you suggested is sufficient probably.

SammyRamone commented 12 months ago

I changed the PR to the the proposed priority ordering of @clalancette. Please re-review it.

SammyRamone commented 8 months ago

friendly ping @clalancette I also fixed a missing sign off in the last commit.

SammyRamone commented 8 months ago

I also added a draft PR for documenting this PR https://github.com/ros2/ros2_documentation/pull/4061

SammyRamone commented 5 months ago

friendly ping @fujitatomoya @wjwwood

SammyRamone commented 4 months ago

friendly ping @wjwwood @fujitatomoya I would be happy to get this finally merged to be able to merge https://github.com/ros2/ros2_documentation/pull/4061 As far as I can see it the failing check is a problem of Jenkin itself with some missing flake8 package.

SammyRamone commented 3 months ago

I would be really happy if we could move this PR forward. Its been almost a year since I created it.

SammyRamone commented 3 weeks ago

Can we somehow get forward with this PR, please? @wjwwood @fujitatomoya @adityapande-1995 @methylDragon @clalancette

SammyRamone commented 3 days ago

The failing checks are just an issue from the CI and I don't know why it says "changes requested", as I incorporated all changes that were requested. As far as I see, this PR is ready to merge. It just needs someone to give it another review. This PR also still blocks https://github.com/ros2/ros2_documentation/pull/4061 @wjwwood @fujitatomoya @adityapande-1995 @methylDragon @clalancette