Requires slightly newer version of msgpack-c about mmtf-cpp HOT 7 CLOSED

Hey @JarrettSJohnson

This is a documentation error, we are linked currently to a particular msgpack git-SHA instead of a release now... Perhaps I should roll us back to a git-SHA associated with the 2.1.5 release.

Do you have any problems with upgrading to a newer msgpack version (the 2.1.5 release)? or is 2.1.1 a dealbreaker for you?

from mmtf-cpp.

@gtauriello do you have anything against locking to 2.1.5?

3.0.1 came out, and we are compatible with that (at least... all tests pass)

from mmtf-cpp.

I don't mind using 2.1.5. If it's ok with @JarrettSJohnson to use that version, I will just update the README here to require the newer version.

Otherwise, I guess we would have to revert to using MSGPACK_ZONE_ALIGN instead (not sure exactly what the effect of that change would be though).
@danpf the MSGPACK_ZONE_ALIGNOF macro was only used in object_encoders.hpp. Any comment on what it does? It does not seem to be particularly well documented in msgpack docs (unless I missed it).

from mmtf-cpp.

it's a wrapper for the compiler function of __alignof__, I think if it's a problem we could just switch to that instead.

https://gcc.gnu.org/onlinedocs/gcc/Alignment.html

https://github.com/msgpack/msgpack-c/blob/0a65c443a260e835e36b4d94b00468d9df1551a2/include/msgpack/v1/detail/cpp11_zone_decl.hpp#L29

I just changed the call to __alignof__ and all the tests pass... but I really don't know enough about compiler object alignments to say if this is a good idea or not.

from mmtf-cpp.

2.1.5 is fine and works. Thanks all.

from mmtf-cpp.

@danpf given the define in msgpack, I guess that alignof doesn't work in Windows...

Anyways: let's just stick to msgpack 2.1.5 and all good. I was going to just modify the README here, but we can do it with your other pull request. Makes sense indeed to lock the submodule versions to some release tag. I will close this here once we merged pull request #8.

from mmtf-cpp.

Ok. Pull request #8 is closed so we can close this one too.
@JarrettSJohnson thanks again for pointing it out.

from mmtf-cpp.