-
Notifications
You must be signed in to change notification settings - Fork 170
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
Fixes for Windows #811
Fixes for Windows #811
Conversation
Codecov Report
@@ Coverage Diff @@
## master #811 +/- ##
==========================================
- Coverage 74.96% 74.87% -0.09%
==========================================
Files 42 42
Lines 3283 3308 +25
==========================================
+ Hits 2461 2477 +16
- Misses 822 831 +9
Continue to review full report at Codecov.
|
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.
One comment about the change in exit code on error. If I was certain the exit code should be non-zero I would make the change myself and merge the PR but I want to get feedback from @Tobias-Fischer to make sure there's not some reason the program should exit cleanly here.
All done now - would be great if you could have another look @nuclearsandwich |
Thanks @Tobias-Fischer! |
This PR contains a minimal set of changes from #661 to get the RoboStack installer (#810) to work on Windows.
@nuclearsandwich already had a look at #661 and the last comment was to rework the Azure pipeline to use GitHub actions instead; in this PR I'm taking a more minimal approach and only pull in easy-to-review changes without touching tests etc. which we don't need for our RoboStack use-case.
The contents of the PR are already used within the conda-forge build of rosdep (see https://github.com/conda-forge/rosdep-feedstock). We confirmed that Windows works in our moveit PR to add cross-platform CI (moveit/moveit#2604) which makes use of rosdep.
/cc @wolfv @traversaro