refactor: drop property_tree dependency - #547
Conversation
|
Boost dependency footprint vs Header-inclusion weights (graph files pulling each direct dependency in):
Transitive Boost modules: 65 → 49 (-16)
|
|
Compiler-warning counts vs
|
dd803bf to
8d16ad5
Compare
8d16ad5 to
520413c
Compare
520413c to
16aca6d
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (98.66%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
16aca6d to
222f7e6
Compare
222f7e6 to
28001be
Compare
28001be to
2f3c407
Compare
|
It's not a refactor because of the change to the exception type. That's not a blocker, it just means we need to treat this change appropriately such as in the release notes make it clear about what the breaking change is. |
|
Anyway, looks like you've got your work cut out to get the coverage back up. :) |
38ea590 to
d707e9f
Compare
|
@jeremy-murphy thank you for the feedback ! Code coverage was actually never 100% so any PR touching uncovered code must cover it, it's cumbersome albeit necessary ahah 😆 Coverage is now up, with a single missing line partially covered, I suggest to leave it as is.
|
d707e9f to
720c137
Compare
Before submitting
developbranch.Type of change
Does this PR introduce a breaking change?
What this PR does
Motivation
property_tree's value is a uniform generic tree with a string-path query API, the same interface we would use for JSON or INI. But GraphML never needs that generality: it only ever asks "tag name? this attribute? this text?", which a rapidxml node answers directly.
Under the hood, property_tree already parses XML with rapidxml (nodes are non owning, pointers to buffer), then copies the entire parse result into its generic ptree so the uniform API works (pure overhead with many allocations as nodes are recursive pairs
string/ptree).This PR keeps the exact same rapidxml parser but walks its DOM directly, deleting the ptree copy (and with it the property_tree dependency).
Testing
The legacy tests properly exercise the round trip and code path to guarantee no regression. The only legacy behavior we can't match is the exception type being raised. Malformed XML now throws
boost::parse_errorinstead of property_tree'sxml_parser_error/ptree_bad_pathBenchmark script to compile and run on develop then on the PR branch:
On develop:
On PR:
Checklist
b2in thetest/directory).