Skip to content

Remove obsolete warnings#31

Merged
clalancette merged 1 commit intofoxy-develfrom
mjeronimo/remove-obsolete-warning
Apr 29, 2021
Merged

Remove obsolete warnings#31
clalancette merged 1 commit intofoxy-develfrom
mjeronimo/remove-obsolete-warning

Conversation

@mjeronimo
Copy link
Copy Markdown

@mjeronimo mjeronimo commented Apr 7, 2021

ros2/rclpy#761 fixes this issue. This PR should be integrated after that one, but not absolutely necessary since this just removes a couple warning messages.

Signed-off-by: Michael Jeronimo michael.jeronimo@openrobotics.org

Remove obsolete warnings
1d19a5f 
ros2/rclpy#761 fixes this issue.

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
@mjeronimo mjeronimo requested a review from sloretz April 7, 2021 23:42
@mjcarroll
Copy link
Copy Markdown

mjcarroll commented Apr 12, 2021

Does it make sense to target dashing-devel with this? The warning is not obsolete in that distro, correct?

@mjeronimo mjeronimo changed the base branch from dashing-devel to master April 15, 2021 15:51
@mjeronimo mjeronimo changed the base branch from master to dashing-devel April 15, 2021 15:52
@mjeronimo
Copy link
Copy Markdown
Author

mjeronimo commented Apr 15, 2021

@mjcarroll Ah, you're right. This should've targeted 'master' as the fix for rclpy was also to master. I tried changing the base branch, but things didn't look good. I can close this PR and open another one since this is a trivial change.

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Apr 17, 2021

@mjeronimo master is for ROS 1, so we'll need a new branch for ros2 if you want to make changes that don't apply to dashing (or crystal, eloquent, foxy, etc...).

I'd recommend ros2 or rolling maybe?

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented Apr 27, 2021

LGTM once this one targets a ros2 or rolling branch.

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Apr 28, 2021

To keep with the "style" of the rest of the rqt plugins, I'm going to go ahead and make a foxy-devel branch and retarget this one to foxy-devel.

@clalancette clalancette changed the base branch from dashing-devel to foxy-devel April 28, 2021 15:05
clalancette
clalancette approved these changes Apr 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me, targeting foxy-devel. I've also opened up ros/rosdistro#29358 , ros2/ros2#1140 , ros2/ros2#1141 , and ros/rosdistro#29359 to switch rolling and galactic over to using this new branch.

This was referenced Apr 29, 2021
Merged
Merged
Merged
Merged
Merged
Merged
@clalancette clalancette merged commit 22d70f6 into foxy-devel Apr 29, 2021
@clalancette clalancette deleted the mjeronimo/remove-obsolete-warning branch April 29, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

@sloretz sloretz sloretz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mjeronimo @mjcarroll @wjwwood @sloretz @clalancette