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

Remove btk references #3280

Merged
merged 8 commits into from Aug 17, 2022

Conversation

AlbertoCasasOrtiz
Copy link
Contributor

@AlbertoCasasOrtiz AlbertoCasasOrtiz commented Aug 16, 2022

Fixes issue -.

Brief summary of changes

Removed all BTK dependencies since now we have switched to ezc3d.

Testing I've completed

Github ci workflows passed in my local fork.

Looking for feedback on...

-.

CHANGELOG.md (choose one)

  • updated.

This change is Reviewable

@AlbertoCasasOrtiz
Copy link
Contributor Author

@aymanhab I have created this new pull request to remove all btk references. Everything is already merged and updated, and I did a deeper search here to find every reference to BTK.

@aymanhab
Copy link
Member

Thanks @AlbertoCasasOrtiz Will definitely review it promptly. Is there a reason you're working off your own fork rather than off a branch? Using a branch allows us to switch to it easily rather than setup a new remote to see the code/changes but I don't know if you have a preference to work off a fork or if you had issues creating a branch off opensim-org/opensim-core directly due to permissions. If the latter please let me know and I'll make sure you can use branches for smoother integration. Thank you.

@AlbertoCasasOrtiz
Copy link
Contributor Author

@aymanhab Sorry about that. I was using a fork so I don't pollute the main repository with tests and new branches. What you are saying seems reasonable to me, so I could work using branches from now on if you prefer.

@aymanhab
Copy link
Member

No worries @AlbertoCasasOrtiz As long as you don't try to merge to master you should be fine experimenting or doing whatever you want or pushing to your own branches on the server, it's only PRs to master that get reviewed. Thank you.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

Reviewed 14 of 21 files at r1, all commit messages.
Reviewable status: 14 of 21 files reviewed, 1 unresolved discussion (waiting on @AlbertoCasasOrtiz and @aymanhab)


CHANGELOG.md line 14 at r1 (raw file):

- Made `Component::getSocketNames` a `const` member method (previously: non-const)
- Modifed the swig interface files to make OpenSim::PathPointSet adopt new PathPoints inserted into it. (Issue #3276)
- Remove BTK dependency and switched completely to ezc3d.

Remove references to obsoleted dependency BTK, use ezc3d exclusively.

@aymanhab aymanhab self-requested a review August 17, 2022 17:15
Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 21 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AlbertoCasasOrtiz)

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

Looks good, just update changlelog for wording. Thank you

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AlbertoCasasOrtiz)

Copy link
Contributor Author

@AlbertoCasasOrtiz AlbertoCasasOrtiz left a comment

Choose a reason for hiding this comment

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

Done, and solved conflict with master.

Reviewable status: 19 of 21 files reviewed, 1 unresolved discussion (waiting on @aymanhab)


CHANGELOG.md line 14 at r1 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

Remove references to obsoleted dependency BTK, use ezc3d exclusively.

Done.

Copy link
Member

@aymanhab aymanhab left a comment

Choose a reason for hiding this comment

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

Thanks @AlbertoCasasOrtiz

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AlbertoCasasOrtiz)

@aymanhab aymanhab merged commit 93ebe03 into opensim-org:master Aug 17, 2022
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

2 participants