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

upstream: ros2_control deprecation replace fake with mock #32

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

moriarty
Copy link
Contributor

This fixes the depreciation warning introduced upstream:
ros-controls/ros2_control@9d5d524

@moriarty moriarty requested review from sea-bass, MarqRazz and eholum and removed request for MarqRazz July 14, 2023 19:26
@moriarty
Copy link
Contributor Author

Found a branch from @sea-bass and opening a Draft PR.

Since this package hasn't been released into humble yet I will try to get this change in.
It is built on top of a branch from @eholum that also hasn't been merged.

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

I think all the URI switches from file:// to package:// are because my original branch was based on a separate moveit_studio branch we have set up.

If you wanna grab these changes, try rebasing on main and dropping the first 2 commits. Sorry.

I also am not sure if this should go into Humble because the parameter changes will break end users' workflows, all for a deprecation warning. If we are cutting a rolling release, I'd be OK with it there though.

@sea-bass
Copy link
Contributor

sea-bass commented Jul 17, 2023

@moriarty branch is rebased on main and contains just the changes we care about.

Again, if we can possibly cut a humble branch before this PR goes in and keep this one going forward as main, that would probably be best in terms of not breaking stable releases.

@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #32 (92567ee) into main (743d20f) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main     #32   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          5       5           
  Lines        352     352           
=====================================
  Misses       352     352           
Flag Coverage Δ
unittests 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@moriarty
Copy link
Contributor Author

@sea-bass yeh that makes sense, I'll first do a 0.0.1 release without this change and then merge this after and create a humble branch.

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.

2 participants