[snapshot] Store metadata using Boost.JSON #4597
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4597 +/- ##
==========================================
+ Coverage 87.19% 87.32% +0.14%
==========================================
Files 249 249
Lines 14276 14261 -15
==========================================
+ Hits 12446 12452 +6
+ Misses 1830 1809 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
898f300 to
c70f357
Compare
c70f357 to
e43fdc9
Compare
ricab
left a comment
There was a problem hiding this comment.
Thanks for the switch and the sprinkled improvements, this makes the code better in a number of places. I have a few comments and questions, but nothing major.
BTW, I see this big warning in boost docs that there is no guarantee of reference stability or iterator stability on insertions and erasures. I don't see that occurring in this PR, but highlighting just in case, for other PRs and other people.
[secondary review]
| boost::json::object update_unique_identifiers_of_metadata(const boost::json::object& metadata, | ||
| const multipass::VMSpecs& src_specs, | ||
| const multipass::VMSpecs& dest_specs, | ||
| const std::string& src_vm_name, | ||
| const std::string& dest_vm_name); |
There was a problem hiding this comment.
Why place this out of the JsonUtils class? Can't mock it that way.
There was a problem hiding this comment.
Maybe this is something to discuss with the team, but one of my goals for our mocks is to keep mocks around only for things that interact with some other part of the system, like I/O (file, network, etc), clocks, virtualization code, etc. This is just a simple pure function, so I don't think we actually need to mock it. By the end of my stack of changes for Boost.JSON, JsonUtils will go away entirely, since all the code will be pure functions or will have been moved elsewhere (e.g. JsonUtils::write_json is now FileOps::write_transactionally).
4d16da7 to
93c066f
Compare
93c066f to
ef40296
Compare
Description
This updates the VM metadata objects in our code to use Boost.JSON instead of Qt's JSON. Since not everything has been converted to use Boost.JSON yet, this temporarily relies on converting back and forth between Boost.JSON and Qt JSON as needed.
Testing
Checklist