override argument parser to fallback to args rather than options#63
override argument parser to fallback to args rather than options#63dirk-thomas merged 4 commits intomasterfrom
Conversation
|
While I picked @mikaelarguedas as the reviewer I hope others will give it a try to and review the patch to move forward on this quickly. |
7c0e15e to
a555aa9
Compare
Codecov Report
@@ Coverage Diff @@
## master #63 +/- ##
==========================================
+ Coverage 77.23% 77.28% +0.05%
==========================================
Files 49 49
Lines 2609 2615 +6
Branches 420 421 +1
==========================================
+ Hits 2015 2021 +6
Misses 571 571
Partials 23 23
Continue to review full report at Codecov.
|
🎉 works great ! Though the arguments following the greedy arguments don't seem to be detected as I expected: For example, here I was expecting a symlink install: Is there some kind of delimimter that should be used ? |
It seemed to work for options following the greedy one in my local testing. Can you double check the logs what args have been parsed.
No delimiter. |
|
Also |
|
Oh yeah that could be it, I took the first non-python package from my topological order ^^ poor choice... Works fine with class_loader 👍 sorry for the noise |
|
I didn't get a chance to test many configurations but, from the few scenarios I tested, this PR works perfectly 👍 |
mikaelarguedas
left a comment
There was a problem hiding this comment.
lgtm, feel free to merge or wait for more testing/feedback from @ros2/team
Fixes #36.
This is an "interesting" approach to monkey patch the change proposed upstream.
I will put this in review for people to try and comment on the patch. Once it is approved the help texts can be updated to mention the space prefix only for cases the the arguments are actually valid options of
colcon.