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

Switch to pnpm #2643

Merged
merged 34 commits into from
Mar 25, 2022
Merged

Switch to pnpm #2643

merged 34 commits into from
Mar 25, 2022

Conversation

preschian
Copy link
Member

@preschian preschian commented Mar 23, 2022

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

PR type

  • Bugfix
  • Feature
  • Refactoring

What's new?

Before submitting Pull Request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've tested it at </rmrk/collection/26902bc2f7c20c546a-1FVG7>
  • I've tested PR on mobile and everything seems works
  • I found edge cases
  • I've written some unit tests 🧪

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

tested in GitHub actions and netlify: https://github.com/preschian/nft-gallery/pull/1/checks
Screen Shot 2022-03-23 at 12 18 57 PM

@netlify
Copy link

netlify bot commented Mar 23, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit bd6d6f6
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/623de43d5589f00009f82f28
😎 Deploy Preview https://deploy-preview-2643--koda-nuxt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@preschian preschian changed the title #2197 Switch to pnpm Switch to pnpm Mar 23, 2022
@preschian
Copy link
Member Author

seems like need to update netlify not to use yarn in the netlify dashboard. it should be able to deploy to netlify, take a look at this PR-test preschian#1

cc @yangwao @vikiival

@preschian preschian marked this pull request as ready for review March 23, 2022 05:36
@preschian preschian marked this pull request as draft March 23, 2022 06:59
@yangwao
Copy link
Member

yangwao commented Mar 24, 2022

what will be build commands?

You can also please update it in, as we have mentioned yarn there, if it builds, I'm happy to change it over to pnpm.

@preschian
Copy link
Member Author

noted, I forgot about the docs. Just realised after doing the #2644 issue 😅
that's why I switch the PR to draft again haha

this is my netlify config, just leave it blank
Screen Shot 2022-03-24 at 7 35 33 PM

@preschian
Copy link
Member Author

preschian commented Mar 24, 2022

suggestion from @yangwao

  • wrap long command in package.json
  • build != generate
  • test it in cloudflare pages

@preschian
Copy link
Member Author

tested in:

improvements:

  yarn pnpm
Netlify Deployment 5m

Screen Shot 2022-03-25 at 2 57 18 PM
3m

Screen Shot 2022-03-25 at 2 54 50 PM
Install Deps in Actions 1m 25s 28s

@preschian preschian marked this pull request as ready for review March 25, 2022 08:13
@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

tested in:

improvements:

  yarn pnpm
Netlify Deployment 5m

Screen Shot 2022-03-25 at 2 57 18 PM 3m

Screen Shot 2022-03-25 at 2 54 50 PM Install Deps in Actions [1m 25s](https://github.com/kodadot/nft-gallery/runs/5676422402?check_suite_focus=true#step:4:35) [**28s**](https://github.com/kodadot/nft-gallery/runs/5687650302?check_suite_focus=true#step:6:467)

impressive time savings.
Curious if you would be keen to write an article on how we've migrated from yarn to pnpm?
Some people might find it interesting as we are still early!
This morning found friends deciding between npm and yarn 😅.

We are usually publishing at https://medium.com/kodadot :)

@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

Once #2668 is resolved, we can check on deploying pnpm

@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

tried with with no build instruction or with npm run generate:pnpm and resulting in same errors

maybe you have something different in your setup?

image

Copy link
Member

@yangwao yangwao left a comment

Choose a reason for hiding this comment

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

maybe adding npm i -g pnpm would help

package.json Outdated Show resolved Hide resolved
netlify.toml Outdated Show resolved Hide resolved
netlify.toml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

I'm confused @preschian

Seems by this log it uses yarn?
however, build instructions are
npm i -g pnpm;npm run generate:pnpm

maybe bc of yarn.lock ? ook, yarn.lock is not present, I'm out of hints.

https://app.netlify.com/sites/koda-nuxt/deploys/623ddcd0b5c58c0008c07402

@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

oh got hint
image

@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

(Netlify Build completed in 3m 50.3s)

Against the previous ~ 4m20 isn't that big saving :|

Cached

4:40:15 PM: (Netlify Build completed in 2m 48.7s)

Bit better! :)

@yangwao yangwao enabled auto-merge March 25, 2022 15:41
@yangwao
Copy link
Member

yangwao commented Mar 25, 2022

Oh apparently need merge something or remove file?

image

@preschian
Copy link
Member Author

@yangwao will update

auto-merge was automatically disabled March 25, 2022 15:46

Head branch was pushed to by a user without write access

@yangwao yangwao merged commit 065920e into kodadot:main Mar 25, 2022
@preschian preschian deleted the switch-pnpm branch March 25, 2022 16:52
@yangwao
Copy link
Member

yangwao commented Mar 27, 2022

sending upper bound with something extra on top! :)

pay 300 usd

thank you @preschian for your amazing all-around cooperation!

@yangwao
Copy link
Member

yangwao commented Mar 27, 2022

😍 Perfect, I’ve sent the payout
💵 $300 @ 167.64 USD/KSM ~ 1.79 $KSM
🧗 DY4SQF2iD456tH89aQtz5wv1EV3BbSW8wKKuMcwbmXaj1pM
🔗 0xded4881c8c32fc626530860209874b1851c788a020a0ea76ae33c9c00a86efaf

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to the newer package manager
2 participants