-
Notifications
You must be signed in to change notification settings - Fork 128
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 types to rosidl_cli #826
Conversation
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@Mergifyio update |
☑️ Nothing to do
|
Pulls: #826 |
Mind adding
|
oops. Will do |
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@fujitatomoya I have address the errors. should be re-runnable now. |
Signed-off-by: Michael Carlstrom <[email protected]>
@fujitatomoya I made a change to fix the Windows error. |
Signed-off-by: Michael Carlstrom <[email protected]>
@fujitatomoya This CI should be ready to re-run. |
Signed-off-by: Michael Carlstrom <[email protected]>
I see I misunderstood the error but, it should be resolved now. |
cc29d8a should fix the mypy failure. CI: |
This PR is causing regressions on Windows: https://ci.ros2.org/view/nightly/job/nightly_win_rel/3191/testReport/junit/(root)/projectroot/test_cli_extension/ Specifically, the code in https://github.com/ros2/rosidl/pull/826/files#diff-6867a0a0c2e4946ffe6fadc04c32c1e9362a9518bac9ca31629c7276d198a13cR24-R58 is not a direct replacement for what was there before (because it was overriding I'm going to propose a revert PR here. |
This reverts commit c71febc.
Ah I see. I missed setting a default for |
@ahcorde @InvincibleRMC @clalancette sorry about this. I will make sure to run test CI with all dependent packages, it should have been able to detect this |
No description provided.