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

[ICON REQUEST] Added XAI and Zora network icons. #9

Merged
merged 7 commits into from
May 16, 2024

Conversation

DominicF96
Copy link
Collaborator

@DominicF96 DominicF96 commented May 16, 2024

Hello again!

Had issues with conflicts on the previous PR, not sure why, also pretty new with working with forks and such...
Original PR - #8

Deleted bun.lockb, hard to resolve conflicts on binary file;
oven-sh/bun#5486 (comment)

  • Check the repository first to ensure the icon isn't already available.
  • 24x24 frame icons respecting the icon size guide
  • Provide the name of the token, a link to the official logo (if available), and any other relevant details.
    • XAI Network, included branded and mono variants
    • Sora Network, only included branded variant since mono is just a plain white disk 😬

@DominicF96 DominicF96 requested a review from 0xa3k5 May 16, 2024 15:59
@DominicF96 DominicF96 changed the title Dominic f96 feature/xai zora network icons [ICON REQUEST] Added XAI and Zora network icons. May 16, 2024
@DominicF96 DominicF96 removed the request for review from 0xa3k5 May 16, 2024 16:03
@DominicF96 DominicF96 marked this pull request as draft May 16, 2024 16:03
@DominicF96
Copy link
Collaborator Author

DominicF96 commented May 16, 2024

Posting conversation here for reference;

Constant paths are relative, so running bun run packages/utils/src/scripts/populate-network-metadata.ts from root doesn't work.

Other scripts in packages/utils/src/scripts/ rely on it so not touching constants file

Running bun run ./src/scripts/populate-network-metadata.ts from /packages/utils, I get an error, Picasso in Gecko Network doesn't have an ID.

Updated script so ID is optional in the meantime (not sure how you define IDs & their impact)

// populate-network-metadata.ts line 40
const lowerCaseId = network.id?.toLowerCase() || ''

Script ran successfully.

Then running bun run build from root worked, but Picasso Network might still be broken.

Last commit may have been a mistake

@0xa3k5
Copy link
Owner

0xa3k5 commented May 16, 2024

hey, thank you.

bun run packages/utils/src/scripts/populate-network-metadata.ts this is actually meant to be run inside the packages/utils (obviously starting from the scripts/...) sorry I said from the root. There is a chance that we could make everything runnable from the root directory tho, I'll think about that.

the "id" field should be mandatory, idk how coin-gecko could have a network without an id but it's a must for us. So I updated the file to use GeckoNetworks interface from packages/utils/src/types.ts and moved the picasso to custom-networks.ts so we could add an id to it.

also updated the script so it doesn't produce empty strings on short-name but instead passes an undefined.

I need to update the ReadMe and Contributing guides for the whole process 😅

@0xa3k5
Copy link
Owner

0xa3k5 commented May 16, 2024

I am also going to pause the github action, it has trouble checking out and committing, I'll reactive it once I fix it. sorry for the annoyance

@0xa3k5 0xa3k5 merged commit ddb9abf into main May 16, 2024
1 check was pending
@0xa3k5 0xa3k5 deleted the DominicF96-feature/xai-zora-network-icons branch May 16, 2024 17:20
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.

2 participants