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

Update getNFTokenID to properly handle binary blob #2247

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Conversation

JST5000
Copy link
Contributor

@JST5000 JST5000 commented Mar 28, 2023

High Level Overview of Change

Update getNFTokenID to allow for binary blobs since that's a valid return from a tx request.

Context of Change

When updating this test to work with the new standalone container, I realized that I had misunderstood the return type and it was more complicated than just the meta in JSON format.

Must be merged in after #2223 as it relies on that docker container for tests.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)

Before / After

  • Migrates the integration tests to use standalone node instead of testnet
  • Expands the function to accept a binary blob as well

Test Plan

CI Passes

Base automatically changed from update-docker to main April 6, 2023 18:11
@mvadari
Copy link
Collaborator

mvadari commented Apr 10, 2023

This PR needs to be rebased before it can be reviewed

@JST5000
Copy link
Contributor Author

JST5000 commented Apr 10, 2023

This PR needs to be rebased before it can be reviewed

I may not get to this for a bit, working on other things which have to be completed this week. Will ping again once this PR is ready to actually be reviewed.

@JST5000 JST5000 requested review from ckniffen and mvadari May 25, 2023 20:28
@ckniffen ckniffen requested a review from jonathanlei May 26, 2023 02:10
@JST5000 JST5000 requested review from mvadari and khancode May 26, 2023 22:05
@XRPLF XRPLF deleted a comment from JST5000 Jun 2, 2023
@XRPLF XRPLF deleted a comment from JST5000 Jun 2, 2023
@JST5000 JST5000 requested a review from mvadari June 12, 2023 23:53
@justinr1234
Copy link
Collaborator

@JST5000 please fix merge conflicts

@JST5000 JST5000 merged commit dc51e3a into main Jun 23, 2023
17 checks passed
@JST5000 JST5000 deleted the update-get-nftokenid branch June 23, 2023 23:17
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.

None yet

4 participants