Sponsored Content
Skip to content

refactor: Remove dead fetchBatch code - #7309

Merged
bthomee merged 2 commits into
developfrom
bthomee/batch
May 21, 2026
Merged

refactor: Remove dead fetchBatch code#7309
bthomee merged 2 commits into
developfrom
bthomee/batch

Conversation

@bthomee

@bthomee bthomee commented May 21, 2026

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

This change removes the fetchBatch functionality, as it was unused.

Context of Change

The fetchBatch functionality was introduced in 1b49776, but the code was actually never used.

Copilot AI review requested due to automatic review settings May 21, 2026 16:55

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the unused fetchBatch NodeStore API and its backend implementations, simplifying the NodeStore backend interface and reducing dead code paths.

Changes:

  • Removed DatabaseNodeImp::fetchBatch implementation and declaration.
  • Removed Backend::fetchBatch pure-virtual API from the public NodeStore backend interface.
  • Deleted fetchBatch overrides from RocksDB, NuDB, Memory, and Null backend implementations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/libxrpl/nodestore/DatabaseNodeImp.cpp Removes the unused DatabaseNodeImp::fetchBatch implementation.
src/libxrpl/nodestore/backend/RocksDBFactory.cpp Removes RocksDBBackend::fetchBatch override.
src/libxrpl/nodestore/backend/NullFactory.cpp Removes NullBackend::fetchBatch override.
src/libxrpl/nodestore/backend/NuDBFactory.cpp Removes NuDBBackend::fetchBatch override.
src/libxrpl/nodestore/backend/MemoryFactory.cpp Removes MemoryBackend::fetchBatch override.
include/xrpl/nodestore/detail/DatabaseNodeImp.h Removes the fetchBatch declaration from the installed DatabaseNodeImp header.
include/xrpl/nodestore/Backend.h Removes the public pure-virtual fetchBatch method from the installed backend interface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/xrpl/nodestore/Backend.h
Comment thread include/xrpl/nodestore/detail/DatabaseNodeImp.h

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@bthomee
bthomee requested review from g-ripple and vlntb May 21, 2026 17:15
@bthomee bthomee added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label May 21, 2026

@vlntb vlntb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bthomee
bthomee enabled auto-merge May 21, 2026 17:16
@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (7fdaa0a) to head (326e4b4).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7309     +/-   ##
=========================================
+ Coverage     82.1%   82.3%   +0.2%     
=========================================
  Files         1011    1011             
  Lines        76387   76190    -197     
  Branches      7411    7309    -102     
=========================================
- Hits         62732   62725      -7     
+ Misses       13655   13465    -190     
Files with missing lines Coverage Δ
include/xrpl/nodestore/Backend.h 14.3% <ø> (ø)
include/xrpl/nodestore/detail/DatabaseNodeImp.h 72.7% <ø> (ø)
src/libxrpl/nodestore/DatabaseNodeImp.cpp 59.3% <ø> (+18.2%) ⬆️
src/libxrpl/nodestore/backend/MemoryFactory.cpp 79.3% <ø> (+11.7%) ⬆️
src/libxrpl/nodestore/backend/NuDBFactory.cpp 68.3% <ø> (+3.4%) ⬆️
src/libxrpl/nodestore/backend/NullFactory.cpp 22.2% <ø> (+1.5%) ⬆️
src/libxrpl/nodestore/backend/RocksDBFactory.cpp 53.8% <ø> (+3.2%) ⬆️

... and 7 files with indirect coverage changes

Impacted file tree graph

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

@bthomee
bthomee added this pull request to the merge queue May 21, 2026
@bthomee bthomee added this to the 3.2.0 milestone May 21, 2026
@bthomee bthomee added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 21, 2026
Merged via the queue into develop with commit 1a98182 May 21, 2026
3 checks passed
@bthomee
bthomee deleted the bthomee/batch branch May 21, 2026 18:30
shortthefomo added a commit to shortthefomo/rippled that referenced this pull request May 22, 2026
Kassaking7 pushed a commit to Kassaking7/rippled that referenced this pull request Jun 2, 2026
Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants