Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Upgrade RapidJSON #6170

Merged
merged 2 commits into from
Sep 15, 2016
Merged

Upgrade RapidJSON #6170

merged 2 commits into from
Sep 15, 2016

Conversation

kkaefer
Copy link
Contributor

@kkaefer kkaefer commented Aug 26, 2016

RapidJSON 1.1.0 was released and has some goodies:

  • C++11 range loop syntax
  • Comments(!) and trailing commas, as well as NaN/Infinity
  • Reduced memory usage

@mention-bot
Copy link

@kkaefer, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh, @mikemorris and @ansis to be potential reviewers.

@kkaefer kkaefer force-pushed the 6170-upgrade-rapidjson branch 2 times, most recently from f508896 to 87a63a1 Compare September 1, 2016 14:00
@kkaefer
Copy link
Contributor Author

kkaefer commented Sep 1, 2016

/cc @brunoabinader @jfirebaugh

@jfirebaugh
Copy link
Contributor

For future reference, can you include a rundown of what the clang-tidy issue was and how we worked around it here or in the commit message?

@kkaefer
Copy link
Contributor Author

kkaefer commented Sep 2, 2016

clang-tidy issue is that the code path for allocating memory in RapidJSON is pretty complicated and I think clang-tidy trips over this. I followed the code path with Xcode's visual analyze feature and think that it is correct. Therefore, I added a .clang-tidy file that disables these checks with in the folder where RapidJSON's write features are used (offline.cpp)

@kkaefer
Copy link
Contributor Author

kkaefer commented Sep 2, 2016

Can't test linking of libgeojson on Android due to #6193. I'll refrain from merging for now.

@kkaefer
Copy link
Contributor Author

kkaefer commented Sep 15, 2016

@brunoabinader @jfirebaugh please review

@kkaefer kkaefer merged commit 5f49f28 into master Sep 15, 2016
@kkaefer kkaefer deleted the 6170-upgrade-rapidjson branch September 15, 2016 13:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants