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 Ubuntu Jammy #474

Merged
merged 3 commits into from
Mar 4, 2022
Merged

add Ubuntu Jammy #474

merged 3 commits into from
Mar 4, 2022

Conversation

ijnek
Copy link
Contributor

@ijnek ijnek commented Mar 3, 2022

Depends on #473 being merged first.

Signed-off-by: Kenji Brameld kenjibrameld@gmail.com

@ijnek ijnek requested a review from a team as a code owner March 3, 2022 02:16
@ijnek ijnek requested review from lihui815 and MichaelOrlov and removed request for a team March 3, 2022 02:16
@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #474 (07963c6) into master (cedd5a2) will decrease coverage by 0.48%.
The diff coverage is 76.92%.

❗ Current head 07963c6 differs from pull request most recent head 7a78d52. Consider uploading reports for the commit 7a78d52 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
- Coverage   93.25%   92.77%   -0.49%     
==========================================
  Files           8        8              
  Lines         163      166       +3     
  Branches       14       15       +1     
==========================================
+ Hits          152      154       +2     
- Misses         11       12       +1     
Impacted Files Coverage Δ
src/setup-ros-linux.ts 97.61% <75.00%> (-2.39%) ⬇️
src/utils.ts 90.32% <75.00%> (-5.33%) ⬇️
src/package_manager/apt.ts 93.75% <100.00%> (+6.25%) ⬆️

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 cedd5a2...7a78d52. Read the comment docs.

@christophebedard
Copy link
Member

Looks like the python package doesn't exist on Jammy, so we should remove python from the list here: https://github.com/ijnek/setup-ros/blob/bc09c616a769d8f4bed93fdda1fd1cca56c77960/src/package_manager/apt.ts#L25. It was moved here for Bionic/Focal: https://github.com/ijnek/setup-ros/blob/bc09c616a769d8f4bed93fdda1fd1cca56c77960/src/package_manager/apt.ts#L35-L66

ijnek added 3 commits March 4, 2022 08:24
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
…ecificAptDependencies

Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
@ijnek
Copy link
Contributor Author

ijnek commented Mar 3, 2022

@christophebedard Thanks for picking that up! That completely slipped by when moving out related changes from #471 . I've rebased, and removed python from aptDependencies.

Copy link
Member

@christophebedard christophebedard 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 PR!

There's one CI job that's still failing, but again it's probably unrelated.

@christophebedard christophebedard merged commit 19d11b7 into ros-tooling:master Mar 4, 2022
@ijnek ijnek deleted the ijnek-add-ubuntu-jammy branch March 4, 2022 00:54
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

2 participants