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

Use self.frame_prefix instead of frame_prefix #205

Merged

Conversation

ggoretkin-bdai
Copy link
Collaborator

@ggoretkin-bdai ggoretkin-bdai commented Dec 5, 2023

All uses of "frame_prefix" are either via self.frame_prefix or self.spot_wrapper.frame_prefix now, reducing potential divergence.

@ggoretkin-bdai
Copy link
Collaborator Author

@ggoretkin-bdai ggoretkin-bdai force-pushed the 12-05-Use_self.frame_prefix_instead_of_frame_prefix_ branch from 5e602e5 to 83af677 Compare December 5, 2023 19:55
@ggoretkin-bdai ggoretkin-bdai force-pushed the 11-21-_N/A_Add_a_mock_launch_argument branch from 9725e97 to 166eb4e Compare December 5, 2023 19:57
@ggoretkin-bdai ggoretkin-bdai force-pushed the 12-05-Use_self.frame_prefix_instead_of_frame_prefix_ branch from 83af677 to 95c47ce Compare December 5, 2023 19:57
@ggoretkin-bdai ggoretkin-bdai marked this pull request as ready for review December 5, 2023 21:53
mhidalgo-bdai
mhidalgo-bdai previously approved these changes Dec 6, 2023
Copy link
Collaborator

@mhidalgo-bdai mhidalgo-bdai left a comment

Choose a reason for hiding this comment

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

LGTM

amessing-bdai
amessing-bdai previously approved these changes Dec 7, 2023
ksharma-bdai
ksharma-bdai previously approved these changes Dec 7, 2023
bhung-bdai
bhung-bdai previously approved these changes Dec 7, 2023
Copy link
Collaborator

@bhung-bdai bhung-bdai left a comment

Choose a reason for hiding this comment

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

HIL tested, fine with me

@ggoretkin-bdai ggoretkin-bdai force-pushed the 12-05-Use_self.frame_prefix_instead_of_frame_prefix_ branch from 95c47ce to d730060 Compare December 7, 2023 18:24
Base automatically changed from 11-21-_N/A_Add_a_mock_launch_argument to main December 8, 2023 18:43
@bhung-bdai bhung-bdai dismissed stale reviews from mhidalgo-bdai, ksharma-bdai, amessing-bdai, and themself December 8, 2023 18:43

The base branch was changed.

@ggoretkin-bdai
Copy link
Collaborator Author

I am not sure why the review approvals are not showing anymore... can @ksharma-bdai and @bhung-bdai re-approve, please?

Copy link
Collaborator

@bhung-bdai bhung-bdai left a comment

Choose a reason for hiding this comment

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

Weird. Reviews got dismissed?

@ggoretkin-bdai
Copy link
Collaborator Author

ggoretkin-bdai commented Dec 13, 2023

Merge activity

  • Dec 13, 10:16 AM: Graphite rebased this pull request as part of a merge.
  • Dec 13, 10:17 AM: Graphite failed to merge this pull request due to The Graphite app does not have permission to push to the target branch. Try your merge again, or report a bug if you see this consistently.

@ggoretkin-bdai ggoretkin-bdai force-pushed the 12-05-Use_self.frame_prefix_instead_of_frame_prefix_ branch from d730060 to 91f3a61 Compare December 13, 2023 15:16
Copy link
Collaborator

@ksharma-bdai ksharma-bdai left a comment

Choose a reason for hiding this comment

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

re-approved!

@bhung-bdai bhung-bdai merged commit d0a52c3 into main Dec 18, 2023
4 checks passed
@bhung-bdai bhung-bdai deleted the 12-05-Use_self.frame_prefix_instead_of_frame_prefix_ branch December 18, 2023 16:07
marlow-fawn pushed a commit to marlow-fawn/spot_ros2 that referenced this pull request Aug 19, 2024
All uses of "frame_prefix" are either via `self.frame_prefix` or `self.spot_wrapper.frame_prefix` now, reducing potential divergence.
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.

5 participants