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

catkin plugin: check stage-snaps for ROS dependencies #2525

Merged

Conversation

Projects
None yet
3 participants
@kyrofa
Copy link
Member

commented Apr 8, 2019

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

Similar to building snaps with a separate underlay (i.e. content sharing), the introduction of stage-snaps means that ROS dependencies can already be satisfied by the time rosdep runs to determine what dependencies are required. This PR resolves LP: #1823788 by reusing the functionality that prevents fetching already-satisfied dependencies in an underlay to ensure that dependencies already satisfied by stage-snaps are treated the same way (i.e. not fetched again).

@kyrofa kyrofa force-pushed the kyrofa:feature/1823788/catkin_stage-snaps branch 3 times, most recently from 488850f to ee32fb8 Apr 8, 2019

catkin plugin: check stage-snaps for ROS dependencies
Similar to building snaps with a separate underlay (i.e. content
sharing), the introduction of `stage-snaps` means that ROS dependencies
can already be satisfied by the time rosdep runs to determine what
dependencies are required. Re-use the functionality that prevents
fetching already-satisfied dependencies in an underlay to ensure that
dependencies already satisfied by stage-snaps are treated the same way
(i.e. not fetched again).

LP: #1823788

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

@kyrofa kyrofa force-pushed the kyrofa:feature/1823788/catkin_stage-snaps branch from ee32fb8 to ae0e94f Apr 8, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Apr 8, 2019

Codecov Report

Merging #2525 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2525      +/-   ##
==========================================
+ Coverage    89.1%   89.11%   +<.01%     
==========================================
  Files         201      201              
  Lines       13650    13656       +6     
  Branches     2065     2067       +2     
==========================================
+ Hits        12163    12169       +6     
  Misses       1048     1048              
  Partials      439      439
Impacted Files Coverage Δ
snapcraft/plugins/catkin.py 94.15% <100%> (+0.08%) ⬆️

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 e39d240...162d6eb. Read the comment docs.

@sergiusens
Copy link
Collaborator

left a comment

Looks good, just a couple of admin comments

kyrofa added some commits Apr 9, 2019

Add type annotations
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
Rename ROS stage-snaps test snap
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>

@sergiusens sergiusens merged commit d1c5f1b into snapcore:master Apr 9, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kyrofa kyrofa deleted the kyrofa:feature/1823788/catkin_stage-snaps branch Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.