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

Adding Ratverse Ascension collection into registry #21

Merged
merged 17 commits into from
Oct 10, 2024
Merged

Conversation

0xyzid
Copy link
Contributor

@0xyzid 0xyzid commented Oct 8, 2024

Adding Ratverse Ascension collection into registry

@ifavo
Copy link
Collaborator

ifavo commented Oct 8, 2024

@0xyzid looks mostly good, except can you check the tokenLink? Its goal is have link to each token on the entry, for example a deep link to a single token on World of V of BlackVeMarket. Right now it links to the example for VeChainStats

update tokenLink
update tokenLink
@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 8, 2024 via email

@ifavo
Copy link
Collaborator

ifavo commented Oct 8, 2024

@0xyzid I believe https://worldofv.art/token/0x1b60b8f4b83d146c051126873b9207247f7676ca/{{tokenId}} would be the correct link for each token on WoV. The {{tokenId}} is replaced with the actual token id, like this: https://worldofv.art/token/0x1b60b8f4b83d146c051126873b9207247f7676ca/957

I believe the TestNet address is incorrect: https://explore-testnet.vechain.org/accounts/0x1b60b8f4b83d146c051126873b9207247f7676ca/

has no contract deployed on it, is that correct?

@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 8, 2024 via email

@ifavo
Copy link
Collaborator

ifavo commented Oct 8, 2024

sorry for the slow reply, I have updated the PR.

the tokenLink is now hard coded and all tokens would link to 957 – can you make it dynamic like https://worldofv.art/token/0x1b60b8f4b83d146c051126873b9207247f7676ca/{{tokenId}} ?

@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 8, 2024 via email

@ifavo
Copy link
Collaborator

ifavo commented Oct 8, 2024

@0xyzid the checks failed, because the folder is not all lower case, can you make the address lower case?

0x1b60b8f4b83d146c051126873b9207247f7676ca

… because you might encounter issues with your local filesystem, you might need to do this in two steps:

  • rename to something totally different like: _0x1b60b8f4b83d146c051126873b9207247f7676c
  • commit
  • rename to 0x1b60b8f4b83d146c051126873b9207247f7676ca
  • commit

because some filesystems might not detect the case changes

@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 8, 2024 via email

@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 8, 2024 via email

@ifavo
Copy link
Collaborator

ifavo commented Oct 8, 2024

@0xyzid Sorry to have two more feedbacks:

  • the extra json has a , too much, which makes the json invalid
  • Bildschirmfoto 2024-10-08 um 22 10 30

I believe you have a typo in your info.json:

    "name": "Ratverse Ascenion",

should it be Ratverse Ascension ?

update line 5
update line 2
@0xyzid
Copy link
Contributor Author

0xyzid commented Oct 9, 2024

apologies on the error, i have updated as per feedback

@ifavo
Copy link
Collaborator

ifavo commented Oct 9, 2024

@libotony this looks good to me now, can you have another look?

@libotony
Copy link
Member

Looks good to me!

@ifavo ifavo merged commit c2e0e89 into vechain:main Oct 10, 2024
4 checks passed
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.

3 participants