Skip to content

Update deprecated usage of get_package_share#3703

Open
stephanie-eng wants to merge 5 commits intomoveit:mainfrom
stephanie-eng:seng/update-get-package-share
Open

Update deprecated usage of get_package_share#3703
stephanie-eng wants to merge 5 commits intomoveit:mainfrom
stephanie-eng:seng/update-get-package-share

Conversation

@stephanie-eng
Copy link
Contributor

Description

CI appears to be borked. Maybe this will help.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (848c062) to head (a3d95b1).

Files with missing lines Patch % Lines
moveit_core/utils/src/robot_model_test_utils.cpp 0.00% 12 Missing ⚠️
moveit_ros/planning/rdf_loader/src/rdf_loader.cpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #3703       +/-   ##
==========================================
- Coverage   46.24%   0.00%   -46.23%     
==========================================
  Files         726     539      -187     
  Lines       59483   47596    -11887     
  Branches     7624    6085     -1539     
==========================================
- Hits        27504       0    -27504     
- Misses      31813   47596    +15783     
+ Partials      166       0      -166     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stephanie-eng stephanie-eng requested a review from nbbrooks March 13, 2026 20:13
@stephanie-eng stephanie-eng force-pushed the seng/update-get-package-share branch 2 times, most recently from be406a2 to f9eb433 Compare March 13, 2026 21:09
@stephanie-eng stephanie-eng force-pushed the seng/update-get-package-share branch from f9eb433 to f0b9544 Compare March 13, 2026 21:34
@nbbrooks
Copy link
Contributor

nbbrooks commented Mar 15, 2026

This is to address ament/ament_index#112

No, this is to address ament/ament_index#104 in Rolling distro - ament/ament_index#112 is not released to debians yet.

{
try
{
return std::filesystem::path(ament_index_cpp::get_package_share_directory(package_name));
Copy link

@Timple Timple Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

return ament_index_cpp::get_package_share_path(package_name);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the review - I moved to this pattern in #3705

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized ament/ament_index#112 is not released to debians yet and the CI breakage Stephanie is referring to is ament/ament_index#104

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping not too many folks would run into this.
It was a strange syntax and deviation from python. Therefor I rushed the new syntax.
Still people affected I see, bummer.

Net syntax does save you some lines in the future though :)

try
{
package_path = ament_index_cpp::get_package_share_directory(package_name);
ament_index_cpp::get_package_share_directory(package_name, path);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path = ament_index_cpp::get_package_share_path(package_name);

@nbbrooks
Copy link
Contributor

@stephanie-eng could you turn on "Allow edits from maintainers" so I can push some changes? For now I duplicated this PR in #3705

@stephanie-eng
Copy link
Contributor Author

Screenshot_20260315_134006_Firefox
Should already be on @nbbrooks

nbbrooks and others added 2 commits March 15, 2026 22:55
This reverts commit f0b9544.
Qt6 migration will be handled in a separate branch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Temporarily disable deprecation warning-as-error for QT and ament_target_dependencies deprecation warnings
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.

3 participants