Refactor JSON Parsing to Use Boost.JSON #202
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CharityKathure
force-pushed
the
JSON-boost-parsing
branch
from
2045e4c to
5c8401d
Compare
CharityKathure
changed the title
CharityKathure
requested review from
TinaMor,
billywr,
bobsira,
iankingori and
profnandaa
TinaMor
reviewed
Mar 12, 2025
Contributor
|
A bit late to the BootJSON migration party, firstly can you point me to any docs we have if any to support the move to use BoostJSON? How efficient is BoostJSON, does it impact Log Monitor in any way, i.e making a bit bulky? Secondly can you provide some sample JSON configuration I can use to test this on my end as I look at the code? I'm assuming the config file does not change much |
TinaMor
reviewed
Mar 18, 2025
TinaMor
reviewed
Mar 18, 2025
CharityKathure
force-pushed
the
JSON-boost-parsing
branch
from
70fad1e to
4c619c0
Compare
Signed-off-by: Charity Kathure <ckathure@microsoft.com>
CharityKathure
force-pushed
the
JSON-boost-parsing
branch
from
cda8a3b to
510d60b
Compare
TinaMor
reviewed
Mar 24, 2025
Signed-off-by: Charity Kathure <ckathure@microsoft.com>
CharityKathure
force-pushed
the
JSON-boost-parsing
branch
from
f414b60 to
548cfca
Compare
TinaMor
reviewed
Apr 3, 2025
bobsira
reviewed
Apr 9, 2025
bobsira
reviewed
Apr 9, 2025
|
|
||
| #include "pch.h" | ||
|
|
||
| #include "../src/LogMonitor/ConfigFileParser.cpp" |
Contributor
There was a problem hiding this comment.
Do we not need this anymore?
Contributor
Author
There was a problem hiding this comment.
We don't need it anymore since we are using the new file JsonProcessor.cpp
bobsira
reviewed
Apr 9, 2025
Comment on lines
-24
to
-26
| <ClCompile Include="ConfigFileParserTests.cpp"> | ||
| <Filter>Source Files</Filter> | ||
| </ClCompile> |
Contributor
There was a problem hiding this comment.
Same question, why are we getting rid of this?
Contributor
Author
There was a problem hiding this comment.
In favour of JsonProcessorTests.cpp
added 2 commits
April 14, 2025 23:41
Signed-off-by: Charity Kathure <ckathure@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant changes to the
LogMonitorproject, primarily focusing on replacing theConfigFileParserandJsonFileParserwith a newJsonProcessormodule. This includes the addition of new functions for handling different log types and updating the project files accordingly.New
JsonProcessormodule:JsonProcessor.cppwith functions to load, parse, and process JSON configuration files, handling different log types such as EventLog, File, ETW, and Process logs.JsonProcessor.hwith declarations for the new functions inJsonProcessor.cpp.Project file updates:
LogMonitor.vcxprojto includeJsonProcessor.cppandJsonProcessor.h, and removed references toConfigFileParser.cppandJsonFileParser.cpp. [1] [2]LogMonitorTests.vcxprojto includeJsonProcessorTests.cppand removed references toConfigFileParserTests.cpp. [1] [2]Test updates:
JsonProcessorTestsinJsonProcessorTests.cppto test the newJsonProcessorfunctionalities.LogMonitorTests.cppto remove the inclusion ofConfigFileParser.cpp.Pipeline changes:
Codebase simplification:
Main.cppto useloadAndProcessJsoninstead ofOpenConfigFilefor reading the configuration file.These changes aim to enhance the functionality and maintainability of the
LogMonitorproject by introducing a more robust JSON processing module.