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

chore: migrate from NPM to Yarn 3 #50

Merged
merged 43 commits into from
Nov 25, 2021
Merged

chore: migrate from NPM to Yarn 3 #50

merged 43 commits into from
Nov 25, 2021

Conversation

antouhou
Copy link
Collaborator

@antouhou antouhou commented Nov 22, 2021

Issue being fixed or feature implemented

NPM has many downsides comparing with Yarn

  • NPM workspaces require to specify current versions in dependencies which makes release process very tricky
  • NPM install is very slow
  • Yarn 3 provides many convenient features

What was done?

  • Switched from NPM to Yarn 3
  • Simplified docker build logic
  • Use Yarn constraints instead of syncpack to validate workspaces
  • Use corepack
  • Defined resolutions to stick on specific deps

How Has This Been Tested?

Manually and with CI

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@antouhou antouhou changed the title Feat yarn2 feat!: yarn2 Nov 23, 2021
@shumkov shumkov changed the base branch from v0.22-dev to master November 24, 2021 04:57
@shumkov shumkov changed the title feat!: yarn2 chore: migrate to yarn2 Nov 24, 2021
@shumkov shumkov changed the title chore: migrate to yarn2 chore: migrate from NPM to Yarn 3 Nov 24, 2021
@@ -15,6 +15,6 @@ on:
jobs:
dapi-tests:
name: Run DAPI gRPC tests
uses: dashevo/platform/.github/workflows/test.yml@master
uses: dashevo/platform/.github/workflows/test.yml@feat-yarn2
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this needs to change to master before merge?

Copy link
Member

Choose a reason for hiding this comment

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

Yep!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you can't rename it before merge, as master will have the correct file only after the merge? @shumkov

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but before we need to make sure that tests are running, which is not a case atm.

@@ -49,7 +48,7 @@ jobs:
return (tag.includes('dev') ? `${major}.${minor}-dev` : 'latest');

- name: Publish NPM packages
run: npm publish --workspaces --access public --tag ${{ steps.tag.outputs.result }}
run: yarn npm publish --access public --tag ${{ steps.tag.outputs.result }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will this actually work?

Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I didn't try it yet :)

Copy link
Member

Choose a reason for hiding this comment

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

Now it should work.

README.md Outdated
- Run `yarn setup` to install dependencies and configure and build all packages
- Run `yarn start` to start the local dev environment built from the sources
- Run `yarn test` to run the whole test suite (note that running tests requires a running node,
so be sure to call `npm start` first). Alternatively, you can run tests for a specific
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yarn start

Copy link
Member

Choose a reason for hiding this comment

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

Thank you. Changed all npm mentions.

@shumkov shumkov added this to the v0.21.x milestone Nov 24, 2021
@shumkov shumkov marked this pull request as ready for review November 24, 2021 22:32
@shumkov shumkov merged commit f940561 into master Nov 25, 2021
@shumkov shumkov deleted the feat-yarn2 branch November 25, 2021 20:53
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