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

Fix/side param #23

Merged
merged 7 commits into from
Apr 24, 2021
Merged

Fix/side param #23

merged 7 commits into from
Apr 24, 2021

Conversation

PetervDooren
Copy link
Contributor

Merge together with tue-robotics/tue_robocup#1083

@PetervDooren PetervDooren marked this pull request as draft February 19, 2021 20:17
@PetervDooren PetervDooren self-assigned this Feb 19, 2021
@PetervDooren PetervDooren mentioned this pull request Feb 19, 2021
Merged
@PetervDooren PetervDooren marked this pull request as ready for review February 23, 2021 19:36
MatthijsBurgh
MatthijsBurgh requested changes Feb 23, 2021
View reviewed changes
Copy link
Member

@MatthijsBurgh MatthijsBurgh left a comment

Choose a reason for hiding this comment

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

Can we get rid of left as well?

MatthijsBurgh
MatthijsBurgh previously approved these changes Mar 21, 2021
View reviewed changes
@PetervDooren @MatthijsBurgh
Relative namespacing
fa318f6 
Co-authored-by: Matthijs van der Burgh <[email protected]>
MatthijsBurgh
MatthijsBurgh previously approved these changes Apr 6, 2021
View reviewed changes
jlunenburg
jlunenburg requested changes Apr 10, 2021
View reviewed changes
Copy link
Contributor

@jlunenburg jlunenburg left a comment

Choose a reason for hiding this comment

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

Since the gripper controller is hero-specific (it depends on tmc_manipulation_msgs and tmc_control_msgs), please move it to 'hero_skills' instead of 'robot_skills'.

@PetervDooren
Copy link
Contributor Author

This is the hero_bridge pr. A lot of things must be moved but that is not inn the scope of this pr

@jlunenburg
Copy link
Contributor

This is the hero_bridge pr. A lot of things must be moved but that is not inn the scope of this pr

My apologies, I have mistaken it for the tue_robocup one. In that case, I'll approve it based on Matthijs' review (it just caught my eye in my mailbox).

jlunenburg
jlunenburg previously approved these changes Apr 10, 2021
View reviewed changes
@PetervDooren PetervDooren dismissed stale reviews from jlunenburg and MatthijsBurgh via a483f3c April 24, 2021 11:12
@PetervDooren PetervDooren merged commit 151d2b0 into master Apr 24, 2021
@MatthijsBurgh MatthijsBurgh deleted the fix/side_param branch December 1, 2021 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatthijsBurgh MatthijsBurgh MatthijsBurgh approved these changes

@LarsJanssenTUe LarsJanssenTUe Awaiting requested review from LarsJanssenTUe

@jlunenburg jlunenburg Awaiting requested review from jlunenburg

Assignees

@PetervDooren PetervDooren

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PetervDooren @jlunenburg @MatthijsBurgh