Skip to content

Conversation

@jcarpent
Copy link
Contributor

@jcarpent jcarpent commented Dec 5, 2019

@wxmerkt I have implemented the full export of the project. It should work now. Could you check that the export is working well?

Before merging and making a new release, I will implement a small unit test to check the export is working well too.

@wxmerkt
Copy link
Member

wxmerkt commented Dec 5, 2019

Thank you for the great effort. I will check it either tomorrow or Monday (and also check that it doesn't break MoveIt)

@jcarpent
Copy link
Contributor Author

jcarpent commented Dec 5, 2019

@wxmerkt I've added a unit test of using the new export. It seems to work well.

@jcarpent
Copy link
Contributor Author

jcarpent commented Dec 9, 2019

@wxmerkt A kind reminder. If you have time today to check this PR ;)

@wxmerkt
Copy link
Member

wxmerkt commented Dec 9, 2019

I have just tested it and it appears to work.

However, please note: The install path for the eigenpyConfig.cmake is changing from share/cmake/eigenpy to lib/cmake/eigenpy (!!).

@jcarpent
Copy link
Contributor Author

jcarpent commented Dec 9, 2019

The install path for the eigenpyConfig.cmake is changing from share/cmake/eigenpy to lib/cmake/eigenpy (!!).

Is it an issue?

@wxmerkt
Copy link
Member

wxmerkt commented Dec 9, 2019

It appears to work - we should note it in case someone comes back in future if it breaks something. From me, this PR is good to go

@jcarpent
Copy link
Contributor Author

jcarpent commented Dec 9, 2019

Do you want me to install it in the share folder too?

@wxmerkt
Copy link
Member

wxmerkt commented Dec 9, 2019

No, let's leave it as is (no duplication) until we have a use case that requires it

@jcarpent
Copy link
Contributor Author

jcarpent commented Dec 9, 2019

@wxmerkt Thanks a lot for your nice feedback.

@jcarpent jcarpent merged commit b20044c into stack-of-tasks:devel Dec 9, 2019
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.

2 participants