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

External wrench smoothing feature for externalwrench gazebo yarp plugin #495

Merged
merged 9 commits into from
Jun 26, 2020
Merged

External wrench smoothing feature for externalwrench gazebo yarp plugin #495

merged 9 commits into from
Jun 26, 2020

Conversation

yeshasvitirupachuri
Copy link
Member

This is a similar PR as #430

It introduces the feature of smoothing the wrench when using externalwrench gazebo yarp plugin

@CLAassistant
Copy link

CLAassistant commented Jun 24, 2020

CLA assistant check
All committers have signed the CLA.

@traversaro
Copy link
Member

Can you make sure that all the commits corresponds to email associated to your GitHub account for the CLA? Thanks a lot!

@traversaro
Copy link
Member

@prashanthr05 can you check if your review from #430 has been addressed? Thanks!

@traversaro
Copy link
Member

Can you update the changelog: https://github.com/robotology/gazebo-yarp-plugins/blob/devel/CHANGELOG.md ? Thanks!

@yeshasvitirupachuri
Copy link
Member Author

Can you make sure that all the commits corresponds to email associated to your GitHub account for the CLA? Thanks a lot!

I am trying to fix it.
The problem seems to be due to the git config user.name I set which is as Yeshasvi Tirupachuri where as the original commits were done with user.name set as Yeshasvitvs

Any pointers to how to fix this ?

@traversaro
Copy link
Member

I would rebase all the commits together, and then change the author with git commit --amend --author, see https://makandracards.com/makandra/1717-git-change-author-of-a-commit .

@yeshasvitirupachuri
Copy link
Member Author

Given #430 (comment) may be @GiulioRomualdi is also interested in reviewing this PR

@prashanthr05
Copy link
Contributor

@prashanthr05 can you check if your review from #430 has been addressed? Thanks!

Yes, it has been addressed. But in the previous review, I had not verified the math. However, it looks like the PR was tested and seems to have been working properly.

I will go through the link added as documentation and cross-verify if required. Please let me know.

@traversaro
Copy link
Member

I think it is ok to approve the PR @prashanthr05 even if you do not review the math in detail.

Copy link
Contributor

@prashanthr05 prashanthr05 left a comment

Choose a reason for hiding this comment

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

Generally, LGTM. But it would be more appropriate to get approval from someone who has extensively tested this PR.

@yeshasvitirupachuri
Copy link
Member Author

Some checks are failing due to robotology/robotology-superbuild#420 which is addressed in #496

@traversaro
Copy link
Member

Hi @Yeshasvitvs , let me know if you prefer to curate the commits, or if it is ok that I merge with squash.

@yeshasvitirupachuri
Copy link
Member Author

if it is ok that I merge with squash.

Please go ahead

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.

None yet

4 participants