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

🎉 initial version of constants for kodadot #110

Merged
merged 7 commits into from
Mar 4, 2023
Merged

Conversation

vikiival
Copy link
Member

@vikiival vikiival commented Mar 3, 2023

Thank you for your contribution to the KodaDot NFT gallery.
👇 _ Let's make a quick check before the contribution.

PR type

  • Fix
  • Feature
  • Refactoring
  • Chore

What's new?

@vikiival vikiival self-assigned this Mar 3, 2023
@vikiival
Copy link
Member Author

vikiival commented Mar 3, 2023

Please if I forgot something, pls let me know.
Or if you advise any other data type or so.

@roiLeo
Copy link
Contributor

roiLeo commented Mar 3, 2023

gh workflow won't work inside static should be at root level

Not sure if it's a good idea to centralized config in a fixed package version.

What would be the steps if we need to make a quick update on relay chain because one of them is dead? (same stuff for squid version upgrade/downgrade)

@vikiival
Copy link
Member Author

vikiival commented Mar 3, 2023

gh workflow won't work inside static should be at root level

fixed in #111

What would be the steps if we need to make a quick update on relay chain because one of them is dead?

Not sure what you mean?

@roiLeo
Copy link
Contributor

roiLeo commented Mar 3, 2023

Not sure what you mean?

Let's say new squid rmrk2 (rc1) out, to make progress on developments we will have to wait for a new package release.
And what if now the squid endpoint broke then new package release to fallback previous endpoint, update the package version on marketplace and then update it again a few days later.

or I extrapolate a bit

static/src/endpoints.ts Outdated Show resolved Hide resolved
static/test/index.test.ts Outdated Show resolved Hide resolved
static/src/chains.ts Show resolved Hide resolved
@vikiival
Copy link
Member Author

vikiival commented Mar 3, 2023

Let's say new squid rmrk2 (rc1) out, to make progress on developments we will have to wait for a new package release.
And what if now the squid endpoint broke then new package release to fallback previous endpoint, update the package version on marketplace and then update it again a few days later.

The intention of the package is to make some constants static something that would not change. For development purposes I would definitely extend package on frontend.

Second things is that we have a ton of obsolete code on the frontend.

Copy link
Contributor

@Jarsen136 Jarsen136 left a comment

Choose a reason for hiding this comment

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

LGTM

@preschian
Copy link
Member

preschian commented Mar 4, 2023

Not sure what you mean?

we will have to wait for a new package release

I only know two methods to address this

  1. use pnpm workspaces. this is what we use with @kodadot1/brick. it means, we need to merge the packages repo inside the nft-gallery. https://pnpm.io/workspaces#referencing-workspace-packages-through-aliases. we can use it directly without waiting to release. and we can publish the packages
  2. because npm does not support installing GitHub URLs with subfolders. we need to turn folders into branches first. https://stackoverflow.com/a/69365523. after that, we can install the package npm install https://github.com/username/repo#branch. I think we need to set up gh-actions for this. to update the branch every time there is a commit into the main branch. (actually, this is similar with wait for the release)

@vikiival
Copy link
Member Author

vikiival commented Mar 4, 2023

@preschian @roiLeo if it makes more sense for you i can move it under the nft-gallery so it will be awailable directly in the workspace.

I just need these constants across the other packages (uniquery, vuex-options)

WDYT?

@preschian
Copy link
Member

move it under the nft-gallery

hhmm, maybe try this first? and let's see what happens next

@vikiival vikiival merged commit 4aa9947 into main Mar 4, 2023
@vikiival vikiival deleted the magic-static branch March 20, 2023 16:10
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.

4 participants