Skip to content

Use Boost::json in olp-cpp-sdk-authentication#1674

Merged
rustam-gamidov-here merged 2 commits intofeature_boost_jsonfrom
rga/ocmam-443-boost-json-to-auth
Mar 3, 2026
Merged

Use Boost::json in olp-cpp-sdk-authentication#1674
rustam-gamidov-here merged 2 commits intofeature_boost_jsonfrom
rga/ocmam-443-boost-json-to-auth

Conversation

@rustam-gamidov-here
Copy link
Collaborator

Migrating from RapidJSON

Relates-To: OCMAM-443

@codecov
Copy link

codecov bot commented Mar 2, 2026

Codecov Report

❌ Patch coverage is 78.51240% with 52 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature_boost_json@678c8f7). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...dk-authentication/src/AuthenticationClientImpl.cpp 73.68% 23 Missing and 7 partials ⚠️
...k-authentication/src/AuthenticationClientUtils.cpp 75.86% 1 Missing and 6 partials ⚠️
olp-cpp-sdk-authentication/src/BaseResult.cpp 68.75% 1 Missing and 4 partials ⚠️
...p-sdk-authentication/src/ResponseFromJsonBuilder.h 78.95% 1 Missing and 3 partials ⚠️
...pp-sdk-authentication/src/SignInUserResultImpl.cpp 69.23% 0 Missing and 4 partials ⚠️
...p-cpp-sdk-authentication/src/TokenEndpointImpl.cpp 89.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##             feature_boost_json    #1674   +/-   ##
=====================================================
  Coverage                      ?   80.38%           
=====================================================
  Files                         ?      351           
  Lines                         ?    14018           
  Branches                      ?     1518           
=====================================================
  Hits                          ?    11267           
  Misses                        ?     2134           
  Partials                      ?      617           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rustam-gamidov-here rustam-gamidov-here force-pushed the rga/ocmam-443-boost-json-to-auth branch 10 times, most recently from a3a9336 to 80cc913 Compare March 2, 2026 14:44
Copy link
Contributor

@andrey-kashcheev andrey-kashcheev left a comment

Choose a reason for hiding this comment

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

Do we need to have JSON sources to be included here in auth as well? It's currently in core, but does it work with visibility=hidden?

@rustam-gamidov-here
Copy link
Collaborator Author

rustam-gamidov-here commented Mar 3, 2026

Do we need to have JSON sources to be included here in auth as well? It's currently in core, but does it work with visibility=hidden?

Nice catch! Looks like it should be in all projects. Not sure how to verify on CI because in tests we are accessing not exposed classes.
Locally added set (CMAKE_CXX_VISIBILITY_PRESET hidden) to the top-level CMakeLists.txt and got several unresolved external symbols building olp-cpp-sdk-authentication. And no unresolved symbols reported with this magic header added to the authentication project.
Tried on macos clang, because previously unresolved symbols were mainly met on clang builds.

Migrating from RapidJSON

Relates-To: OCMAM-443
Signed-off-by: Rustam Gamidov <ext-rustam.gamidov@here.com>
Migrating from RapidJSON

Relates-To: OCMAM-443
Signed-off-by: Rustam Gamidov <ext-rustam.gamidov@here.com>
@rustam-gamidov-here rustam-gamidov-here force-pushed the rga/ocmam-443-boost-json-to-auth branch from e362140 to 787a8ee Compare March 3, 2026 12:17
@rustam-gamidov-here rustam-gamidov-here merged commit 4b553db into feature_boost_json Mar 3, 2026
24 checks passed
@rustam-gamidov-here rustam-gamidov-here deleted the rga/ocmam-443-boost-json-to-auth branch March 3, 2026 12:29
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