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 resolve_center_of_pressure into its own file #7367

Conversation

jwnimmer-tri
Copy link
Collaborator

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

Relates #6464, per cleaning up `util.


This change is Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

+@amcastro-tri for feature review, please.

@jwnimmer-tri jwnimmer-tri force-pushed the util-resolve_center_of_pressure branch 2 times, most recently from cb7db41 to 91f661e Compare October 29, 2017 14:44
@amcastro-tri
Copy link
Contributor

:lgtm:


Reviewed 10 of 10 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


drake/util/drakeUtil.h, line 200 at r1 (raw file):

void baseZeroToBaseOne(std::vector<int>& vec);

double angleAverage(double theta1, double theta2);

I think you can now also safely remove this file (and the cpp) since the two methods above are not even used anywhere.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

drake/util/drakeUtil.h, line 200 at r1 (raw file):

Previously, amcastro-tri (Alejandro Castro) wrote…

I think you can now also safely remove this file (and the cpp) since the two methods above are not even used anywhere.

OK #7364 removes those two methods.

Also note the ~180 LOC above the fold above those two methods, that is not dead yet. (The std::vector to Eigen::Matrix to cVector conversion routines are quite popular.)


Comments from Reviewable

@jwnimmer-tri jwnimmer-tri force-pushed the util-resolve_center_of_pressure branch from 91f661e to 793cd91 Compare October 30, 2017 15:11
@amcastro-tri
Copy link
Contributor

Review status: 9 of 10 files reviewed at latest revision, 1 unresolved discussion.


drake/util/drakeUtil.h, line 200 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

OK #7364 removes those two methods.

Also note the ~180 LOC above the fold above those two methods, that is not dead yet. (The std::vector to Eigen::Matrix to cVector conversion routines are quite popular.)

Ah, good point, I was only judging by the cpp contents.


Comments from Reviewable

@amcastro-tri
Copy link
Contributor

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

+@EricCousineau-TRI for platform review per rotation, please.

@EricCousineau-TRI
Copy link
Contributor

:lgtm:


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


Comments from Reviewable

@jwnimmer-tri jwnimmer-tri merged commit 0bdfc22 into RobotLocomotion:master Oct 30, 2017
@jwnimmer-tri jwnimmer-tri deleted the util-resolve_center_of_pressure branch October 30, 2017 16:05
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