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

Update README to use lerna to install all deps #1724

Closed
wants to merge 2 commits into from

Conversation

nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Jul 8, 2020

Are there other lerna-related instructions to install dependencies that we should add to the repo's documentation? For example, now that we are using yarn does the user still need to check the LTS release npm?

Signed-off-by: Nick Pai npai.nyc@gmail.com

Signed-off-by: Nick Pai <npai.nyc@gmail.com>
Signed-off-by: Nick Pai <npai.nyc@gmail.com>
@@ -9,7 +9,7 @@ The following steps require the `git` CLI to be installed. If you are on Windows
Clone the UMA [repo](https://github.com/UMAprotocol/protocol). Start in the top-level directory in this repository, `protocol/`.

1. Install the latest stable version of [Node.js](https://nodejs.org/) and ensure that `npm` is installed along with it.
Copy link
Member Author

Choose a reason for hiding this comment

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

Reiterating this question:

"For example, now that we are using yarn does the user still need to check the LTS release npm?"

Copy link
Member

Choose a reason for hiding this comment

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

It depends on a few things:

  • Are we still planning on using npx to run things in node_modules/.bin or will we be using some sort of yarn equivalent?
  • Will lerna be installed globally using yarn or using npm? Do we even need lerna in these tutorials or can we just use yarn to do the install?
  • (not sure if this is relevant for the tutorial) will all of the package.json scripts work the same with yarn run as they do with npm run?

Copy link
Member

Choose a reason for hiding this comment

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

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the npx equivalent for yarn is just running yarn x. yarn x can execute x that is in ./node_modules/.bin, as shown here.

For example, here you can see the yarn truffle is the same as $(npm bin)/truffle, which are both different from my local globally installed truffle:

➜  core git:(npai/lerna-voter-dapp) yarn truffle version
yarn run v1.22.0
$ /Users/nicholaspai/UMA/protocol/node_modules/.bin/truffle version
Truffle v5.1.33 (core: 5.1.33)
Solidity v0.5.16 (solc-js)
Node v13.8.0
Web3.js v1.2.1
✨  Done in 1.57s.
➜  core git:(npai/lerna-voter-dapp) $(npm bin)/truffle version
Truffle v5.1.33 (core: 5.1.33)
Solidity - 0.6.6 (solc-js)
Node v13.8.0
Web3.js v1.2.1
➜  core git:(npai/lerna-voter-dapp) truffle version
Truffle v5.1.15 (core: 5.1.15)
Solidity - 0.6.6 (solc-js)
Node v13.8.0
Web3.js v1.2.1

Copy link
Member Author

Choose a reason for hiding this comment

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

Regarding the equivalent of running npx lerna bootstrap, you can just run lerna bootstrap.

I see on the lerna docs that they recommend running npx lerna bootstrap, but I'm having trouble locating in our code where exactly npm is aware of a repo-installed lerna. Did I install lerna globally or does yarn come with it?

Copy link
Member

Choose a reason for hiding this comment

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

My assumption is that you've run npm install -g lerna in the past. When I first ran lerna, it wasn't available in my environment.

Something work trying to double check: npm uninstall -g lerna. That should get rid of it if you have it installed globally through npm.

Copy link
Member

Choose a reason for hiding this comment

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

It would be my preference to not use npx if we can replace everything with equivalent yarn commands. The only place in the docs where I see npx mentioned is when they run npx lerna init

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.725% when pulling f459f67 on npai/lerna-docs into 1675c9a on master.

Copy link
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

Looks great! One question

@@ -9,7 +9,7 @@ The following steps require the `git` CLI to be installed. If you are on Windows
Clone the UMA [repo](https://github.com/UMAprotocol/protocol). Start in the top-level directory in this repository, `protocol/`.

1. Install the latest stable version of [Node.js](https://nodejs.org/) and ensure that `npm` is installed along with it.
Copy link
Member

@mrice32 mrice32 Jul 8, 2020

Choose a reason for hiding this comment

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

Should we add some additional prerequisites? Like installing yarn and lerna? I think we'll definitely need yarn since lerna uses it to do the installation under the hood. (Lerna could technically just be used with npx without installing, but that makes the calls much slower)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should change our docs to reflect that we're using yarn now not npm, but its a bit confusing to have both npx and yarn present.

@nicholaspai nicholaspai marked this pull request as draft July 9, 2020 13:36
@chrismaree chrismaree deleted the npai/lerna-docs branch December 19, 2022 07:56
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