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

Move non-general utilities into robotInterfaces QP controllers #7363

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Oct 29, 2017

Relates #6464, in that it's difficult to make //drake/util well-phrased with so much going on in there.

All of the method declarations and definitions are just cut-paste of code (+ nolint tags). The using-statments, includes, and BUILD rules are fixed up by hand.


This change is Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

+@siyuanfeng-tri for feature review, please.

@jwnimmer-tri jwnimmer-tri force-pushed the util-robotInterfaces branch 3 times, most recently from f8b308f to 096edb1 Compare October 30, 2017 18:15
It's only used from within controllers, and shouldn't be used elsewhere.
We mark it private, since it's only used from .cc files, also.
It's only used from within controllers, and shouldn't be used elsewhere.
We mark it private, since it's only used from .cc files, also.
@siyuanfeng-tri
Copy link
Contributor

:lgtm: thanks @jwnimmer-tri


Reviewed 25 of 25 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

jwnimmer-tri commented Nov 1, 2017

+@ggould-tri for platform review per rotation, please.

@ggould-tri
Copy link
Contributor

:lgtm:


Reviewed 2 of 25 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jwnimmer-tri jwnimmer-tri merged commit 544e30b into RobotLocomotion:master Nov 1, 2017
@jwnimmer-tri jwnimmer-tri deleted the util-robotInterfaces branch November 1, 2017 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants