Skip to content
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 action send_goal prototype completer #301

Merged

Conversation

artivis
Copy link
Contributor

@artivis artivis commented Jul 17, 2019

Add an action goal prototype completer to ros2 action send_goal.
E.g.

$ ros2 action send_goal /fibonacci action_tutorials/action/Fibonacci [tab][tab]
-f          --feedback  order:\ 0\ 

Not very pretty but one can at least figure out the first few characters of the goal body.
Then,

$ ros2 action send_goal /fibonacci action_tutorials/action/Fibonacci "o [tab]

resolves to,

$ ros2 action send_goal /fibonacci action_tutorials/action/Fibonacci "order: 0"

This follows the work initiated in #298.

Signed-off-by: artivis jeremie.deray@canonical.com

@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch from 4bc829a to 09f6a08 Compare July 17, 2019 19:10
@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch 2 times, most recently from 44cff92 to cf4649b Compare July 26, 2019 20:42
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I left a minimal comment.

ros2action/ros2action/api/__init__.py Outdated Show resolved Hide resolved
@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch from cf4649b to 553b9b5 Compare August 2, 2019 20:00
@artivis
Copy link
Contributor Author

artivis commented Aug 2, 2019

@ivanpauno I'll wait on your feedback and final approval of this PR before updating #299 & #300.

@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch 2 times, most recently from 6fb5b87 to cea6e59 Compare August 2, 2019 21:37
@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch 2 times, most recently from 1b2bea4 to 421cba8 Compare August 6, 2019 20:03
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution!
I manually tested it, and it works well.

Sanity check:

  • Linux: Build Status

@ivanpauno
Copy link
Member

  • Linux: Build Status

@artivis There's a linter failure, could you address it? Thanks!

@artivis
Copy link
Contributor Author

artivis commented Aug 7, 2019

@artivis There's a linter failure, could you address it? Thanks!

Sure thing !

Signed-off-by: artivis <jeremie.deray@canonical.com>
@artivis artivis force-pushed the feature/action-send-goal-proto-completer branch from 421cba8 to 1eeba50 Compare August 7, 2019 12:36
@artivis
Copy link
Contributor Author

artivis commented Aug 7, 2019

@ivanpauno Should be fixed now.
I didn't realized that I had a broken version of flake8-docstrings .

@ivanpauno
Copy link
Member

@ivanpauno Should be fixed now.
I didn't realized that I had a broken version of flake8-docstrings .

I tested locally, and linter is passing (and auto-completion continues working well).
I'm merging this, thanks for the PR @artivis.

@ivanpauno ivanpauno merged commit 27f2102 into ros2:master Aug 7, 2019
@ivanpauno
Copy link
Member

@artivis another minimal comment: try to avoid force pushing if it's not completely necessary, it does the review process harder.

@kyrofa kyrofa mentioned this pull request Sep 4, 2019
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants