rippled icon indicating copy to clipboard operation
rippled copied to clipboard

refactor: remove `Json::Object` and related files/classes

Open mvadari opened this issue 3 months ago • 3 comments

High Level Overview of Change

This PR removes include/xrpl/json/Object.h and all downstream files. There are a number of minor downstream changes as well.

Full list of deleted classes and functions:

  • Json::Collections
  • Json::Object
  • Json::Array
  • Json::WriterObject
  • Json::setArray
  • Json::addObject
  • Json::appendArray
  • Json::appendObject

The last helper function, copyFrom, seemed a bit more complex and was actually used in a few places, so it was moved to LedgerToJson.h instead of deleting it.

Context of Change

Json::Object and related objects are not used at all - probably some piece of legacy code that was forgotten about.

Type of Change

  • [x] Refactor (non-breaking change that only restructures code)

API Impact

N/A - no change in behavior, pure refactor

Test Plan

Json::Object-specific tests were removed, and all other tests pass.

mvadari avatar Oct 15 '25 17:10 mvadari

Codecov Report

:x: Patch coverage is 80.39216% with 10 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 79.1%. Comparing base (f816ffa) to head (f254c69). :warning: Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/app/ledger/detail/LedgerToJson.cpp 72.2% 5 Missing :warning:
src/xrpld/rpc/handlers/LedgerHandler.cpp 72.2% 5 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #5894    +/-   ##
========================================
  Coverage     79.1%   79.1%            
========================================
  Files          839     836     -3     
  Lines        71386   71245   -141     
  Branches      8343    8319    -24     
========================================
- Hits         56447   56353    -94     
+ Misses       14939   14892    -47     
Files with missing lines Coverage Δ
include/xrpl/protocol/ApiVersion.h 100.0% <100.0%> (ø)
include/xrpl/protocol/ErrorCodes.h 100.0% <ø> (ø)
src/libxrpl/protocol/ErrorCodes.cpp 90.3% <100.0%> (+4.6%) :arrow_up:
src/libxrpl/protocol/RPCErr.cpp 100.0% <100.0%> (ø)
src/xrpld/app/ledger/LedgerToJson.h 100.0% <ø> (ø)
src/xrpld/rpc/Status.h 100.0% <100.0%> (ø)
src/xrpld/rpc/handlers/Version.h 100.0% <100.0%> (ø)
src/xrpld/app/ledger/detail/LedgerToJson.cpp 74.9% <72.2%> (-1.3%) :arrow_down:
src/xrpld/rpc/handlers/LedgerHandler.cpp 25.6% <72.2%> (+5.2%) :arrow_up:

... and 8 files with indirect coverage changes

Impacted file tree graph

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Oct 15 '25 17:10 codecov[bot]

@mvadari do you want help merging from develop to your branch ?

Bronek avatar Nov 07 '25 16:11 Bronek

No new changes after my last review. Waiting for @mvadari to update the PR.

pratikmankawde avatar Nov 10 '25 16:11 pratikmankawde