-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Don't use unique_ptr on pre-c++11 branch #238
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
Conversation
in case of embedded nulls
This partially reverts commit 45cd949. Ignored ValueInternal* changes, since those did not produce symbols for Debian build. (They must not have used the INTERNAL stuff.) Ignored CZString changes since those are private (and sizeof struct did not change). open-source-parsers#78 Conflicts: include/json/value.h src/lib_json/json_internalarray.inl src/lib_json/json_internalmap.inl src/lib_json/json_value.cpp
for binary-compatibility with 0.6.0 issue open-source-parsers#147 was open-source-parsers#57
…character" This reverts commit 5bf1610.
re: 28836b8 A global instance of a Value (viz. 'null') was a mistake, but dropping it breaks binary-compatibility. So we will keep it everywhere except the one platform where it was crashing, ARM.
…on non-standard JSON.' revert '642befc836ac5093b528e7d8b4fd66b66735a98c', but keep the *added* methods for `decodedNumber()` and `decodedDouble()`.
Don't use C++11 unique_ptr in the 0.y.z branch. Although this usage is guarded with __cplusplus >= 201103 some build configurations (notably chromium) use a C++11-compliant compiler but a pre-11 library.
I'm confused. How can |
It does sound strange doesn't it. Because chromium supports a lot of platforms, C++11 support across the codebase can only be fully supported once all the target platforms support C++11. The current status is here: https://chromium-cpp.appspot.com/. I think most of the C++11 language features are now allows but so far none of the library (libstdc++/libc++) features. To quote from that page "In particular, chromium/android is currently using STLport, and chromium/mac is currently using libstdc++4.2, which predate C++11.". I figured since this branch was specifically for pre-C++11 builds that you might be OK with this change. I not, we can patch it in chromium. Or perhaps you would be OK with a different macro to guard this? |
Ugh! That's maddening. I understand why they would want language features without the libraries -- to facility the migration to C++11 -- but they need to provide an extra macro. Ok. We'll pull your commit on the |
Don't use C++11 unique_ptr in the 0.y.z branch. Although this usage is guarded with __cplusplus >= 201103 some build configurations (notably chromium) use a C++11-compliant compiler but a pre-11 library. pull #238
Branch commit is fine. We will pin to a sha1. |
Hmm, strange, did you end up re-writing history on the 0.y.z branch? Was that deliberate? I makes things difficult for existing forks when that happens. |
I see it looks like for each 0.10.X release you are doing a rebase onto master (is that right?). Would it be possible to instead merge master into 0.y.x each time to avoid re-writing history in the future? |
The I'd rather not start merging. It creates too much of a mess. But I could instead cherry-pick changes from Let me think about this... My first thought is that you could base your fork branch on the |
I agree that its nicer to have a linear history for your 0.y.z branch that always sits nicely on top of your master branch. However, I don't think history re-writing is ever a good idea in a public repo. I had trouble with my fork locally and it also caused the chromium mirror of jsoncpp (https://chromium.googlesource.com/external/github.com/open-source-parsers/jsoncpp/) to stop working (this is probably fairly chrome specific since the mirroring code doesn't allow for 'push -f'). My advise is to live with merging when the branch is public. The other downside to merging is that it makes comparing 0.10.1 and 0.10.2 hard. You end up seeing the all the commits twice: Also, even with merging it should be very obvious what the patches are: Just advice. I don't mean to tell you how to run your project. |
I hadn't considered I need a rebase-able branch, but it doesn't have to be the one releases come from. So I'll start a new branch, How far back do you need to go? Re-create |
Just for the future would be great. Thanks! |
Don't use C++11 unique_ptr in the 0.y.z branch. Although this usage is guarded with __cplusplus >= 201103 some build configurations (notably chromium) use a C++11-compliant compiler but a pre-11 library. pull open-source-parsers#238
Don't use C++11 unique_ptr in the 0.y.z branch. Although this usage is guarded with __cplusplus >= 201103 some build configurations (notably chromium) use a C++11-compliant compiler but a pre-11 library. pull open-source-parsers#238
Don't use C++11 unique_ptr in the 0.y.z branch.
Although this usage is guarded with __cplusplus >= 201103
some build configurations (notably chromium) use a
C++11-compliant compiler but a pre-11 library.