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

Using nlohmann::json instead of RapidJson #145

Merged
merged 1 commit into from
Dec 11, 2017
Merged

Conversation

khabinov
Copy link
Contributor

@khabinov khabinov commented Dec 7, 2017

No description provided.

@khabinov khabinov requested review from mikea and parsifal-47 December 7, 2017 02:53
@parsifal-47
Copy link
Contributor

why do we need libcbor? I thought the whole point was to remove it completely as well

Copy link
Contributor

@parsifal-47 parsifal-47 left a comment

Choose a reason for hiding this comment

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

We still have leaks with libcbor, it is not clear how would we solve them. The original idea was to get rid of both libraries at a time, because nlohmann::json has support for cbor as well. It looks like there is no point to invest our time in libcbor improvements

@khabinov
Copy link
Contributor Author

khabinov commented Dec 8, 2017

why do we need libcbor? I thought the whole point was to remove it completely as well

Can't do it right away nlohmann/json#862. At least, we get a better json library.

@khabinov khabinov force-pushed the oleg/nlohmann-json branch 5 times, most recently from 4656e4e to c4a7dd7 Compare December 8, 2017 21:29
@khabinov khabinov mentioned this pull request Dec 8, 2017
@parsifal-47
Copy link
Contributor

ok, sorry, I thought it is possible to replace our cbor library with this one

@khabinov khabinov merged commit de77eef into master Dec 11, 2017
@khabinov khabinov deleted the oleg/nlohmann-json branch December 11, 2017 21:24
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.

3 participants