Skip to content

Removed deprecated cancel_sleep_or_wait#2836

Merged
ahcorde merged 1 commit intorollingfrom
ahcorde/rolling/remove_deprecated_clock_cancel_sleep_or_wait
May 2, 2025
Merged

Removed deprecated cancel_sleep_or_wait#2836
ahcorde merged 1 commit intorollingfrom
ahcorde/rolling/remove_deprecated_clock_cancel_sleep_or_wait

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented May 1, 2025

No description provided.

@ahcorde
Removed deprecated cancel_sleep_or_wait
4163537 
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
@ahcorde ahcorde self-assigned this May 1, 2025
fujitatomoya
fujitatomoya approved these changes May 1, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm with green CI

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented May 1, 2025

Pulls: #2836
Gist: https://gist.githubusercontent.com/fujitatomoya/b850865d945611f71f539c2bdc6acd57/raw/405e1a691761d4e2c5d3bd6129ef3fc5a760974f/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp
TEST args: --packages-above rclcpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15877

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit f81caac into rolling May 2, 2025
2 of 3 checks passed
@ahcorde ahcorde deleted the ahcorde/rolling/remove_deprecated_clock_cancel_sleep_or_wait branch May 2, 2025 13:11
@jmachowinski
Copy link
Copy Markdown
Collaborator

jmachowinski commented May 2, 2025

This PR deleted to much tests.

@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented May 2, 2025

This PR deleted to much tests.

I thought this other test https://github.com/ros2/rclcpp/blob/rolling/rclcpp/test/rclcpp/test_clock_conditional.cpp is a replacement.

@jmachowinski
Copy link
Copy Markdown
Collaborator

jmachowinski commented May 6, 2025

I'll double check, and will report back...

@jmachowinski
Copy link
Copy Markdown
Collaborator

jmachowinski commented May 8, 2025 •
edited
Loading

The other test only check the new implementation. With the removal of the old test, the CI does not cover clock->sleep_until any more.

We should be ably to replace everything inside clock->sleep_until with the new clock conditional. Then it would be okay to drop he old tests. I just didn't have time to do this yet...

@fujitatomoya
Copy link
Copy Markdown
Collaborator

fujitatomoya commented May 8, 2025

@jmachowinski thanks for checking this, can you add the test as you mentioned above when you have time? or should we revert this PR until we figure that out?

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

Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees

@ahcorde ahcorde

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ahcorde @fujitatomoya @jmachowinski