Skip to content

Conversation

rwaskiewicz
Copy link
Contributor

Hi there 👋 while playing around with the project, I noticed a small typo in the setup guide. There doesn't appear to be an open issue for it (please let me know if I should create one and reference this one).

While watching npm install run, I noticed that the step for running lerna was part of a postinstall step:

  postinstall
    lerna bootstrap

which led me to removing that as well. If it's still necessary, I'd be happy to restore it :-)

Changes:

  • Update directory name that's created after cloning the project
  • Remove lerna bootstrap step (appears to be run as a postinstall step in npm scripts)

- Update directory name that's created after cloning the project
- Remove lerna bootstrap step (appears to be run as a postinstall step in npm scripts)
@rwaskiewicz rwaskiewicz changed the base branch from master to develop October 13, 2018 17:45
@AllenFang
Copy link
Member

Hello @rwaskiewicz . thanks for fixing the type :)
However, I dont see the commit about removing lerna bootstrap. anyway, I think it's necessary to run the lerna bootstrap, that will install all the external deps for each packages. so any reason why you want to remove the lerna bootstrap after package install?

Thanks

@rwaskiewicz
Copy link
Contributor Author

I think it's necessary to run the lerna bootstrap, that will install all the external deps for each packages. so any reason why you want to remove the lerna bootstrap after package install?

I absolutely agree that it's necessary, my question was that it appears that it automatically runs as a part of an npm install as a postinstall step. Is it needed to have to run it manually after the npm install + first run of the bootstrap is complete? Perhaps I'm missing something where the first install isn't doing something that manually running lerna bootstrap does?

Below is output from only running npm i locally with the project to demonstrate what I'm seeing:

> npm i

> react-bootstrap-table2@0.0.1 postinstall /Dev/react/react-bootstrap-table2
> lerna bootstrap

lerna info version 2.8.0
lerna info versioning independent
lerna info Bootstrapping 7 packages
lerna info lifecycle preinstall
lerna info Symlinking packages and binaries
lerna info lifecycle postinstall
lerna info lifecycle prepublish
lerna info lifecycle prepare
lerna success Bootstrapped 7 packages
npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.

audited 23460 packages in 17.221s
found 0 vulnerabilities

If it's needed still, I'm happy to revert the removal of the bootstrap command :-)

@AllenFang
Copy link
Member

AllenFang commented Oct 14, 2018

@rwaskiewicz I agree it is not very necessary to do the lerna bootstrap after npm install. But it's just a safety way to run it after npm install and also it's much convenient(Remember that lerna bootstrap will install all the package dependencies and links). In addition, it's good that everytime we release, we always run npm install to make sure the local dependencies and package links is updated correctly.

I think there's no any hurts for running the lerna bootstrap after install and actually it's a fast execution.

@rwaskiewicz
Copy link
Contributor Author

@AllenFang Sorry for the delay, some how I missed your response 🤔 Updated the PR to keep the lerna step 🙂

@AllenFang AllenFang merged commit 569c22b into react-bootstrap-table:develop Oct 21, 2018
@AllenFang
Copy link
Member

It's ok, thanks your contribution 👍

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.

2 participants