-
Notifications
You must be signed in to change notification settings - Fork 304
feat(ledger-browser): handle ERC721 token metadata #3894
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great! I've left two small comments.
Also, remember to fix the failing CI tests:
- yarn_codegen
- yarn_lint (
Server
is defined but never used in one of the tests) - commitlint
packages/cactus-plugin-persistence-ethereum/src/main/typescript/plugin-persistence-ethereum.ts
Outdated
Show resolved
Hide resolved
...persistence-ethereum/src/test/typescript/integration/persistence-ethereum-functional.test.ts
Show resolved
Hide resolved
Hi @outSH! Thanks for the feedback. I will address them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@musicboy0322 LGTM assuming @outSH 's comments are addressed.
...persistence-ethereum/src/test/typescript/integration/persistence-ethereum-functional.test.ts
Show resolved
Hide resolved
f50ffc9
to
413b54d
Compare
413b54d
to
7e5b19e
Compare
Hi @outSH ! Thanks for the response! I have updated the code based on the two pieces of feedback. If I misunderstood anything, please let me know and I will address it. Thanks again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. thank you!
Primary Change: * Modify 'TestERC721.json' to update 'BaseUrl' for 'TokenUri()' testing * Add logic to fetch data from 'TokenUri()' * Extend 'name', 'description', and 'image' columns in 'token_erc721' table * Add a simple API server to simulate 'TokenUri()' data responses Fixes hyperledger-cacti#3552 Signed-off-by: musicboy0322 <[email protected]>
Primary Change:
Fixes #3552
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.