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.

Sign up for GitHub

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

Jump to bottom

Replace yarn with npm #7222

Merged
3 commits merged into from
Sep 8, 2020
Merged

Replace yarn with npm #7222

3 commits merged into from
Sep 8, 2020

Conversation

ghost
Copy link

@ghost ghost commented Aug 25, 2020 •
edited by ghost
Loading

We replace yarn with npm and update the daml assistant and compatibility tests accordingly. To update the @daml.js library, just rerun the codegen, npm will pick up the changes automatically.

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@ghost ghost force-pushed the replace_yarn_with_npm branch 23 times, most recently from 2a375bf to b8bd3a2 Compare August 31, 2020 13:37
@ghost ghost force-pushed the replace_yarn_with_npm branch 7 times, most recently from a09e93e to 2e3a08e Compare September 3, 2020 10:21
@ghost ghost force-pushed the replace_yarn_with_npm branch 5 times, most recently from 648ee92 to b2fa770 Compare September 7, 2020 10:31
@ghost ghost marked this pull request as ready for review September 7, 2020 10:33
nemanja-da
nemanja-da approved these changes Sep 7, 2020
View reviewed changes
Copy link
Contributor

@nemanja-da nemanja-da left a comment

Choose a reason for hiding this comment

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

Thnx Robin!

cocreature
cocreature reviewed Sep 7, 2020
View reviewed changes
Copy link
Contributor

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Nice work, thank you!

This also needs to update the release instructions which currently still reference yarn.

compatibility/bazel_tools/create-daml-app/Main.hs Show resolved Hide resolved
compatibility/bazel_tools/create-daml-app/Main.hs Show resolved Hide resolved
compatibility/bazel_tools/create-daml-app/Main.hs Show resolved Hide resolved
compatibility/bazel_tools/create-daml-app/Main.hs Show resolved Hide resolved
compatibility/bazel_tools/create-daml-app/Main.hs Show resolved Hide resolved
templates/create-daml-app/ui/package.json.template Show resolved Hide resolved
docs/source/getting-started/index.rst Show resolved Hide resolved
@nemanja-da
Copy link
Contributor

@cocreature think this is still all WiP, right @Robin-da?

@cocreature
Copy link
Contributor

Screenshot from 2020-09-07 14-10-57
It was a WIP until this morning.

@ghost
Copy link
Author

ghost commented Sep 7, 2020

@nemanja-da No, now it's ready for review.

@ghost ghost force-pushed the replace_yarn_with_npm branch 2 times, most recently from 08ba56e to 3ea3358 Compare September 8, 2020 09:28
hurryabit
hurryabit approved these changes Sep 8, 2020
View reviewed changes
Copy link
Contributor

@hurryabit hurryabit left a comment

Choose a reason for hiding this comment

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

This is awesome. Thanks a lot.

daml-assistant/integration-tests/src/DA/Daml/Assistant/CreateDamlAppTests.hs Outdated Show resolved Hide resolved
templates/create-daml-app/ui/.gitignore Outdated Show resolved Hide resolved
@ghost ghost force-pushed the replace_yarn_with_npm branch from 3cb60f0 to 063bcb5 Compare September 8, 2020 09:40
cocreature
cocreature approved these changes Sep 8, 2020
View reviewed changes
Copy link
Contributor

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Nice work thank you! One comment on the node version which still matters and I agree with @hurryabit wtr to the .gitignore.

docs/source/getting-started/index.rst Show resolved Hide resolved
@ghost ghost force-pushed the replace_yarn_with_npm branch from 063bcb5 to 05a1d03 Compare September 8, 2020 09:50
@ghost ghost merged commit 678a8ee into master Sep 8, 2020
@ghost ghost deleted the replace_yarn_with_npm branch September 8, 2020 10:07
bame-da pushed a commit that referenced this pull request Sep 8, 2020
@bame-da
Replace yarn with npm (#7222)
1381a60 
* replace yarn with npm in docs

CHANGELOG_BEGIN
CHANGELOG_END

* updating assistant and compatibility tests

* moved gitignore entry to toplevel
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aherrmann-da aherrmann-da Awaiting requested review from aherrmann-da

@bame-da bame-da Awaiting requested review from bame-da

@garyverhaegen-da garyverhaegen-da Awaiting requested review from garyverhaegen-da

@sofiafaro-da sofiafaro-da Awaiting requested review from sofiafaro-da

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nemanja-da @cocreature @hurryabit @garyverhaegen-da