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

Switch to use the upstream source code of visit_struct #58

Merged
merged 2 commits into from
Jul 11, 2022

Conversation

traversaro
Copy link
Collaborator

This PR switches to use a commit on the upstream branch of ami-iit's fork of visit_struct. That branch is created by cherry-picking the CMake structured added in ami-iit/visit_struct#1 on the top of the latest commit of the master branch of https://github.com/garbageslam/visit_struct, that got VISITABLE_INIT in https://github.com/garbageslam/visit_struct/pull/24/files .

Using the upstream source code of visit_struct would simplify packaging the library in conda-forge . However, as far as I understood the code is different between https://github.com/garbageslam/visit_struct/pull/24/files and cbeck88/visit_struct#14, so we may want to merge with care. The tests of matio-cpp are passing fine, but I do not know if there is something else we want to check before merging.

@S-Dafarra
Copy link
Member

S-Dafarra commented Jul 11, 2022

Actually, the modification of cbeck88/visit_struct#14 are useful for the weird initialization of

struct nestedStruct
{
using array_3f = std::array<float,3>;
BEGIN_VISITABLES(nestedStruct);
VISITABLE_DIRECT_INIT(array_3f, array, {1.0, 2.0, 3.0});
VISITABLE(testStruct, s);
END_VISITABLES;
int* notSupported = nullptr;
};
. It is not mentioned in the README either. We started using visit_struct in robometry, so we may want to check it keeps compiling too (although I am not expecting big surprises).

@S-Dafarra S-Dafarra merged commit 271b0e4 into master Jul 11, 2022
@S-Dafarra S-Dafarra deleted the useupstream branch April 15, 2023 15:04
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