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.

Sign up for GitHub

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

Jump to bottom

Rename rclpy.qos.QoS*Policy to rclpy.qos.*Policy #1832

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Rename rclpy.qos.QoS*Policy to rclpy.qos.*Policy #1832

merged 1 commit into from
Oct 25, 2024

Conversation

christophebedard
Copy link
Member

Just a simple change. Although these names were not deprecated, they were renamed in ros2/rclpy#379.

@christophebedard christophebedard requested a review from a team as a code owner October 11, 2024 22:40
@christophebedard christophebedard requested review from emersonknapp and jhdcs and removed request for a team October 11, 2024 22:40
MichaelOrlov
MichaelOrlov approved these changes Oct 12, 2024
View reviewed changes
Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

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

@christophebedard LGTM.
However, I am curious how tests were passing with renamed from rclpy.qos import QoSDurabilityPolicy?

fujitatomoya
fujitatomoya approved these changes Oct 12, 2024
View reviewed changes
Copy link
Contributor

@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
Contributor

Pulls: #1832
Gist: https://gist.githubusercontent.com/fujitatomoya/bd1cd9aaaa7a0a6d2fbcc3fd78b006fd/raw/5338f29b39e16f898fceef0b024ad3cf6ed5419e/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2
TEST args: --packages-above rosbag2
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14698

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

@fujitatomoya
Copy link
Contributor

However, I am curious how tests were passing with renamed from rclpy.qos import QoSDurabilityPolicy?

for not breaking the downstream packages, we have an alias for retroccompatibility.

https://github.com/ros2/rclpy/blob/a09a0312cf9e2bbe0f6dc4467c72060a2fb74ce5/rclpy/rclpy/qos.py#L410

@christophebedard
Copy link
Member Author

Yes, these were still working because there are aliases. And using those aliases does not trigger deprecation warnings.

@MichaelOrlov
Copy link
Contributor

Re-run Windows CI build

  • Windows Build Status

@MichaelOrlov MichaelOrlov merged commit 786c3c4 into ros2:rolling Oct 25, 2024
12 checks passed
@MichaelOrlov
Copy link
Contributor

https://github.com/Mergifyio backport jazzy

@mergify Mergify
Copy link

mergify bot commented Oct 25, 2024 •
edited
Loading

backport jazzy

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 25, 2024
@christophebedard @mergify
Rename rclpy.qos.QoS*Policy to rclpy.qos.*Policy (#1832)
95a33e3 
Signed-off-by: Christophe Bedard <[email protected]>
(cherry picked from commit 786c3c4)
@mergify mergify bot mentioned this pull request Oct 25, 2024
Merged
@christophebedard christophebedard deleted the christophebedard/update-rclpy-qos-policy-class-names branch October 28, 2024 18:08
ahcorde pushed a commit that referenced this pull request Oct 28, 2024
@mergify @christophebedard
Rename rclpy.qos.QoS*Policy to rclpy.qos.*Policy (#1832) (#1841)
e07248d 
Signed-off-by: Christophe Bedard <[email protected]>
(cherry picked from commit 786c3c4)

Co-authored-by: Christophe Bedard <[email protected]>
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

@MichaelOrlov MichaelOrlov MichaelOrlov approved these changes

@emersonknapp emersonknapp Awaiting requested review from emersonknapp emersonknapp is a code owner automatically assigned from ros2/tooling-reviewers

@jhdcs jhdcs Awaiting requested review from jhdcs jhdcs is a code owner automatically assigned from ros2/tooling-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@christophebedard @fujitatomoya @MichaelOrlov