refactor: remove Json::Object and related files/classes
#5894
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5894 +/- ##
========================================
Coverage 79.5% 79.5%
========================================
Files 817 814 -3
Lines 72210 72064 -146
Branches 8280 8252 -28
========================================
- Hits 57398 57302 -96
+ Misses 14812 14762 -50
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
include/xrpl/protocol/ErrorCodes.h
Outdated
| template <class JsonValue> | ||
| void | ||
| inject_error(error_code_i code, JsonValue& json) | ||
| inline void |
There was a problem hiding this comment.
nit: This does not actually have to be in the header anymore. I'd move it to the cpp file
include/xrpl/protocol/ErrorCodes.h
Outdated
| void | ||
| inject_error(error_code_i code, std::string const& message, JsonValue& json) | ||
| inline void | ||
| inject_error(error_code_i code, std::string const& message, Json::Value& json) |
There was a problem hiding this comment.
There is probably some reason why this was int. Maybe someone else will be able to remember/explain why
There was a problem hiding this comment.
I assumed it was for Json::Object reasons
| // VFALCO NOTE Deprecated function | ||
| Json::Value | ||
| rpcError(int iError) | ||
| rpcError(error_code_i iError) |
There was a problem hiding this comment.
Is it even still used? says it's deprecated above so maybe possible to remove entirely. Worth checking
There was a problem hiding this comment.
Yes, still used pretty heavily. I didn't want to refactor it out here because it would have expanded the scope a lot, and this PR is pretty wide as-is.
| { | ||
| XRPL_ASSERT( | ||
| from.isObjectOrNull(), "Json::doCopyFrom : valid input type"); | ||
| auto members = from.getMemberNames(); |
There was a problem hiding this comment.
nit: This and below can be const i think
| to = from; | ||
| else | ||
| { | ||
| XRPL_ASSERT( |
There was a problem hiding this comment.
nit: The function is not called doCopyFrom
Signed-off-by: JCW <[email protected]>
…ets merged Signed-off-by: JCW <[email protected]>
2 participants
High Level Overview of Change
This PR removes
include/xrpl/json/Object.hand all downstream files. There are a number of minor downstream changes as well.Full list of deleted classes and functions:
Json::CollectionsJson::ObjectJson::ArrayJson::WriterObjectJson::setArrayJson::addObjectJson::appendArrayJson::appendObjectThe last helper function,
copyFrom, seemed a bit more complex and was actually used in a few places, so it was moved toLedgerToJson.hinstead of deleting it.Context of Change
Json::Objectand related objects are not used at all - probably some piece of legacy code that was forgotten about.Type of Change
API Impact
N/A - no change in behavior, pure refactor
Test Plan
Json::Object-specific tests were removed, and all other tests pass.