Allow more flexible usage of XML QoS profiles#460
Conversation
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
|
@jacobperron I was originally thinking to add this in a fork, but if the defaults aren't modified and flexibility is provided through environment variables, I guess we can merge the change on master. |
|
BTW, this is ready for being reviewed. I opened it as a draft only because tests are missing. |
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com> Co-authored-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
jacobperron
left a comment
There was a problem hiding this comment.
I didn't exercise the feature, but it LGTM. One wording suggestion.
|
@jacobperron I'm planning to write some tests before merging, to improve coverage. |
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
jacobperron
left a comment
There was a problem hiding this comment.
LGTM, just some tiny nitpicks
|
Going in, thanks for the reviews @jacobperron ! |
|
@ivanpauno There is a chance that this introduced flakiness in the nightly Windows builds. Do you mind taking a look and seeing if it was this PR? These are the tests in question: https://ci.ros2.org/job/nightly_win_rel/1721/testReport/ |
I don't see how this PR can make that test fail, it's completely unrelated. Most likely, the test was already flaky. |
|
I didn't think so, but I also hadn't seen these tests pop up in recent history. I'll see if I can reproduce, thanks! |
This reverts commit 2572187.
It allows more flexibility through environment variables, checkout
README.mdfor more details.TODO: Write tests.