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

Update Frame shortcut #958

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Update Frame shortcut #958

merged 2 commits into from
Mar 20, 2023

Conversation

DLu
Copy link
Contributor

@DLu DLu commented Mar 17, 2023

No description provided.

@DLu
Update Frame shortcut
59a09fc 
Signed-off-by: David V. Lu <davidvlu@gmail.com>
@DLu DLu requested a review from ahcorde as a code owner March 17, 2023 13:44
clalancette
clalancette reviewed Mar 17, 2023
View reviewed changes
Copy link
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.

Just so I understand; this changes no logic, but just consolidates some duplicated lines of code into a method. Is that correct?

@DLu
Copy link
Contributor Author

DLu commented Mar 17, 2023

Correct

clalancette
clalancette requested changes Mar 20, 2023
View reviewed changes
Copy link
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.

I've left one style nit to fix, otherwise this looks good to me.

rviz_common/include/rviz_common/display.hpp Outdated Show resolved Hide resolved
@DLu
Formatting
6f941e1 
Signed-off-by: David V. Lu <davidvlu@gmail.com>
@clalancette
Copy link
Contributor

CI:

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

@clalancette clalancette merged commit 5eeae5b into ros2:rolling Mar 20, 2023
@DLu DLu deleted the updateFrame branch March 20, 2023 17:09
@DLu
Copy link
Contributor Author

DLu commented Mar 20, 2023

Backport to foxy/humble?

@clalancette
Copy link
Contributor

Backport to foxy/humble?

I think we could; it doesn't really change API/ABI. On the other hand, it seems like a simple cleanup, so I'm not sure it is worth it. What's your interest in the backport?

@DLu
Copy link
Contributor Author

DLu commented Mar 20, 2023

Cleaning up my new tutorial

@clalancette
Copy link
Contributor

I'll open up the backport to Humble, though I want to have some soak time in Rolling/Iron before we merge this in.

I'm less inclined to do it for Foxy since we are so close to its EOL.

@clalancette
Copy link
Contributor

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Mar 20, 2023 •
edited
Loading

backport humble

✅ Backports have been created

@mergify mergify bot mentioned this pull request Mar 20, 2023
Merged
mergify bot pushed a commit that referenced this pull request Mar 20, 2023
@DLu @mergify
Update Frame shortcut (#958)
d07365a 
* Update Frame shortcut

Signed-off-by: David V. Lu <davidvlu@gmail.com>
(cherry picked from commit 5eeae5b)
@DLu
Copy link
Contributor Author

DLu commented Mar 21, 2023

Thanks @clalancette ! Thanks @Mergifyio!

ahcorde pushed a commit that referenced this pull request Jun 9, 2023
@mergify @DLu
Update Frame shortcut (#958) (#960)
c7fdd81 
* Update Frame shortcut

Signed-off-by: David V. Lu <davidvlu@gmail.com>
(cherry picked from commit 5eeae5b)

Co-authored-by: David V. Lu!! <davidvlu@gmail.com>
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

@ahcorde ahcorde Awaiting requested review from ahcorde ahcorde is a code owner

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@DLu @clalancette