Nlohmann json Implementation #211
Conversation
Signed-off-by: Charity Kathure <[email protected]>
CharityKathure
force-pushed
the
nlohmann-json
branch
from
26f0cab to
72282ce
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR migrates JSON handling from Boost to nlohmann::json, refactors log output sanitization, and updates the build system to CMake with static linking enabled.
- Switch JSON parsing and log sanitization to nlohmann::json for improved readability and maintainability.
- Migrate the build system from MSBuild to CMake and enable static linking to avoid runtime dependency issues.
- Remove Boost JSON usage throughout the codebase and update CI pipelines accordingly.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| build.cmd | Updated to install nlohmann-json and configure CMake-based builds. |
| azure-pipelines.yml | Revised to install nlohmann-json and update platform build configurations. |
| LogMonitor/src/LogMonitor/pch.h | Switched JSON header include from Boost to nlohmann::json. |
| LogMonitor/src/LogMonitor/Utility.cpp | Reworked JSON sanitization utilizing nlohmann::json instead of iterative character replacement. |
| LogMonitor/src/LogMonitor/JsonProcessor.{h,cpp} | Updated JSON processing functions and error handling from Boost to nlohmann::json. |
| LogMonitor/src/CMakeLists.txt | Refactored targets to link with nlohmann_json and configured static library creation. |
| LogMonitor/LogMonitorTests/* | Adjusted test code and CMake configuration to use nlohmann::json. |
| LogMonitor/CMakeLists.txt | Configured static linking, enforced MSVC runtime changes, and migrated to CMake. |
Comments suppressed due to low confidence (1)
LogMonitor/src/LogMonitor/Utility.cpp:277
- [nitpick] The new JSON sanitization approach via json::dump() replaces character-by-character escaping; verify that the resulting escaped string exactly meets the expected log formatting. Ensure that edge cases (e.g. embedded nulls or uncommon escape sequences) are consistently handled across all scenarios as defined in the tests.
json j = utf8;
| file(GLOB_RECURSE SourceFiles RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "*.cpp") | ||
| file(GLOB_RECURSE HeaderFiles RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}" "*.h") | ||
|
|
||
| # Define LogMonitor as a library | ||
| add_library(LogMonitor ${SourceFiles}) | ||
| # Define LogMonitorLib as a static library | ||
| add_library(LogMonitorLib STATIC ${SourceFiles}) |
There was a problem hiding this comment.
The same set of source files is used to build both the static library (LogMonitorLib) and the executable (LogMonitor), potentially leading to duplicate compilation. Consider separating the core sources for the library from those defining the executable's entry point to avoid symbol duplication and reduce build overhead.
CharityKathure
requested review from
TinaMor,
billywr,
bobsira,
danielGithinji,
iankingori and
profnandaa
profnandaa
left a comment
profnandaa
left a comment
There was a problem hiding this comment.
Other that one comment, I'm ok with this.
| utf8.erase(std::find(utf8.begin(), utf8.end(), '\0'), utf8.end()); | ||
|
|
||
| // Escape the string using JSON | ||
| json j = utf8; |
There was a problem hiding this comment.
is this a copy or move constructor? see if there are any optimizations that can be made for memory, I'm sure this has some cost. Same to wstring->string and then string->wstring conversions...
This pull request makes the following changes:
Switches JSON handling from Boost to nlohmann::json
Replaces the previous configuration file parsing logic that used Boost with nlohmann::json for improved readability and maintainability.
Log output sanitization using nlohmann::json
Sanitizes log output using nlohmann::json to ensure consistency and safety in serialized data.
Migrates build system to CMake
Refactors the build.cmd script to replace the MSBuild-based approach with a CMake-based build system for greater portability and modern tooling support.
Enables static linking
Configures the build for static linking to avoid runtime dependency issues on target machines.