Add support for boost::optional #122
Conversation
|
Looks pretty much how it should - needs a few minor touch ups (e.g. comments not matching) and unit tests and then it will be good to go. Will likely merge after 1.1 is wrapped up as part of #123 in 1.2. |
|
I’ll get those addressed in the next several days Steve H. From: Shane Grant [mailto:[email protected]] Looks pretty much how it should - needs a few minor touch ups (e.g. comments not matching) and unit tests and then it will be good to go. Will likely merge after 1.1 is wrapped up as part of #123#123 in 1.2. — |
1) Added unit tests for boost_optional support 2) Added unit test to unittest project 3) alphabetized type includes in common.hpp (so it's easier to tell what is being included) NOTE: project includes reference to boost libraries you may not want to merge in
|
Unit tests (unittest/boost_optional.cpp) added. Comments cleaned up. Steve H. From: Hickman, Steve (AdvTech) I’ll get those addressed in the next several days Steve H. From: Shane Grant [mailto:[email protected]] Looks pretty much how it should - needs a few minor touch ups (e.g. comments not matching) and unit tests and then it will be good to go. Will likely merge after 1.1 is wrapped up as part of #123#123 in 1.2. — |
1) Circular Buffer 2) Compressed Pair 3) Dynamic Bitset
1) filesystem
|
It seems to be missing move semantics in some places, such as in the boost::optional load(): would better be Anyway pretty cool job you are doing on supporting the boost types! |
| } | ||
|
|
||
|
|
||
| //! Loading for boost::optional |
There was a problem hiding this comment.
I think this should say "boost::compressed_pair", not optional
|
Not being able to serialise |
Related to last commit: Added serialisation methods for boost::optional. Copied the file from USCiLab/cereal#122, hopefully that pull-request will be accepted. This commit adds the file to CMakeLists.
|
It is something I want to do in the near future but I want to separate out serialization code for external libraries from the core of cereal, so it won't make the next immediate release but is targeted for the one after that. |
|
@AzothAmmo I do agree with you. However, don't you think |
|
Well we can always bring it in and then push it out to a different place once we do modules. |
|
I would (of course) leave the decision up to you. I can offer to prepare a PR. I just figured however since I include the cereal headers in my projects directly (and not via submodule or external module or something), it actually doesn't really make a difference and I can just include the file for optional in my projects. I'm OK with either. |
|
Per instructions on the web site, I've now made this and other updates on the current develop branch and submitted a new pull request. See PR #282 |
|
It would be awesome to get this merged into the next version. If there's anything I can do to help, let me know. I had to fork Cereal and pull in this boost stuff (along with a tweak to get Cereal working with Android). However, it would be awesome if this was officially supported. |
|
I'll get the boost support synched with the latest develop branch this weekend. |
Copyright notice is identical to your original wording, with my name there instead.
This should be sufficient to support boost::optional