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

Fixes for Windows #811

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

Tobias-Fischer
Copy link
Contributor

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

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #811 (c00d7cf) into master (30a25a7) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
src/rosdep2/installers.py 78.27% <100.00%> (ø)
src/rosdep2/main.py 48.94% <100.00%> (+0.11%) ⬆️
src/rosdep2/platforms/source.py 90.60% <100.00%> (+0.05%) ⬆️
src/rosdep2/shell_utils.py 100.00% <100.00%> (ø)
src/rosdep2/sources_list.py 85.58% <0.00%> (-1.00%) ⬇️
src/rosdep2/lookup.py 87.91% <0.00%> (+0.08%) ⬆️
src/rosdep2/rospkg_loader.py 94.44% <0.00%> (+0.24%) ⬆️
src/rosdep2/catkin_packages.py 67.56% <0.00%> (+0.90%) ⬆️
src/rosdep2/rospack.py 61.76% <0.00%> (+1.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30a25a7...c00d7cf. Read the comment docs.

Copy link
Contributor

@nuclearsandwich nuclearsandwich left a 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.

src/rosdep2/main.py Outdated Show resolved Hide resolved
@Tobias-Fischer
Copy link
Contributor Author

All done now - would be great if you could have another look @nuclearsandwich

@nuclearsandwich
Copy link
Contributor

Thanks @Tobias-Fischer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants