Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional support in ledger_entry #5236

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tequdev
Copy link
Contributor

@tequdev tequdev commented Jan 7, 2025

close #5198

High Level Overview of Change

Context of Change

Type of Change

  • [x ] New feature (non-breaking change which adds functionality)

API Impact

  • [x ] Public API: New feature (new methods and/or new fields)

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

Attention: Patch coverage is 82.22222% with 8 lines in your changes missing coverage. Please review.

Project coverage is 77.9%. Comparing base (0324764) to head (56d631b).

Files with missing lines Patch % Lines
src/xrpld/rpc/handlers/LedgerEntry.cpp 82.2% 8 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5236   +/-   ##
=======================================
  Coverage     77.9%   77.9%           
=======================================
  Files          783     783           
  Lines        66707   66752   +45     
  Branches      8118    8119    +1     
=======================================
+ Hits         51953   51998   +45     
  Misses       14754   14754           
Files with missing lines Coverage Δ
src/xrpld/rpc/handlers/LedgerEntry.cpp 77.0% <82.2%> (+0.9%) ⬆️

... and 2 files with indirect coverage changes

Impacted file tree graph

@mvadari
Copy link
Collaborator

mvadari commented Jan 8, 2025

I think the API for the singletons like amendments deserves a bit more discussion. I'm not entirely convinced that a boolean is the right way to go about it (but I'm also not sure what the right way should be).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add NFT Offer support to ledger_entry
2 participants