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.

Sign up for GitHub

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

Jump to bottom

Nftids #780

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Nftids #780

merged 6 commits into from
Jul 26, 2023

Conversation

cindyyan317
Copy link
Collaborator

@cindyyan317 cindyyan317 commented Jul 19, 2023 •
edited
Loading

Add nftoken_ids or nftoken_id / offer_id to nft transactions.

[----------] Global test environment tear-down
[==========] 835 tests from 82 test suites ran. (57076 ms total)
[ PASSED ] 831 tests.
[ FAILED ] 4 tests, listed below:
[ FAILED ] SubscriptionManagerSimpleBackendTest.SubscriptionManagerLedger
[ FAILED ] RPCNoRippleCheckTest.NormalPathRoleGatewayDefaultRippleUnsetTrustLineNoRippleUnsetHighAccount
[ FAILED ] RPCNoRippleCheckTest.NormalPathTransactions
[ FAILED ] RPCSubscribeHandlerTest.StreamsLedger

@codecov
Copy link

codecov bot commented Jul 19, 2023 •
edited
Loading

Codecov Report

Merging #780 (17e4cfe) into develop (2336148) will increase coverage by 0.14%.
Report is 3 commits behind head on develop.
The diff coverage is n/a.

❗ Current head 17e4cfe differs from pull request most recent head f744c85. Consider uploading reports for the commit f744c85 to get more accurate results

@@             Coverage Diff             @@
##           develop     #780      +/-   ##
===========================================
+ Coverage    34.55%   34.69%   +0.14%     
===========================================
  Files          161      161              
  Lines         8257     8346      +89     
  Branches      4851     4898      +47     
===========================================
+ Hits          2853     2896      +43     
- Misses        2822     2831       +9     
- Partials      2582     2619      +37

see 3 files with indirect coverage changes

cindyyan317
cindyyan317 commented Jul 24, 2023
View reviewed changes
cindyyan317
cindyyan317 commented Jul 24, 2023
View reviewed changes
@cindyyan317 cindyyan317 requested a review from godexsoft July 24, 2023 11:59
@cindyyan317 cindyyan317 marked this pull request as ready for review July 24, 2023 11:59
@intelliot intelliot mentioned this pull request Jul 25, 2023
Merged
7 tasks
@cindyyan317
tmp
32d2751 
tmp

unittest plan

add unittests

tx unittests

add tests

change API

fix

remove unused functions

fix tests

remove empty line
@cindyyan317
conan tmp
5621888
godexsoft
godexsoft requested changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

Leaving a few suggestions, mostly small formatting and styling issues - the code changes look good to me.

@godexsoft godexsoft self-requested a review July 26, 2023 15:32
godexsoft
godexsoft approved these changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@godexsoft godexsoft left a comment

Choose a reason for hiding this comment

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

👍

@cindyyan317 cindyyan317 merged commit 71aabc8 into XRPLF:develop Jul 26, 2023
@sappenin sappenin mentioned this pull request Oct 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@godexsoft godexsoft godexsoft approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cindyyan317 @godexsoft