Skip to content

Updating lerna to latest version #160

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

Merged
merged 3 commits into from
Aug 30, 2018
Merged

Updating lerna to latest version #160

merged 3 commits into from
Aug 30, 2018

Conversation

aliabbasrizvi
Copy link
Contributor

No description provided.

@aliabbasrizvi
Copy link
Contributor Author

@spencerwilson-optimizely recommended to push package-lock.json as well

@aliabbasrizvi
Copy link
Contributor Author

This was an update that was recommended by SourceClear

@coveralls
Copy link

coveralls commented Aug 30, 2018

Coverage Status

Coverage increased (+0.0009%) to 97.114% when pulling 4a5e28f on ali/update_library into 8f9bcb3 on master.

@spencerwilson-optimizely
Copy link
Contributor

spencerwilson-optimizely commented Aug 30, 2018

[javascript] > lerna bootstrap && lerna run build
[javascript] 
[javascript] lerna info version 2.11.0
[javascript] lerna ERR! EMISMATCH Incompatible local version of lerna detected!
[javascript] lerna ERR! EMISMATCH The running version of lerna is 2.11.0, but the version in lerna.json is 3.2.1.
[javascript] lerna ERR! EMISMATCH You can either run 'lerna init' again or install 'lerna@^3.2.1'.
[javascript] npm info lifecycle optimizely-sdk-packages@1.0.0~postinstall: Failed to exec postinstall script

Looks like in the repo's postinstall hook triggered during the testapp's installation step, global lerna detects a mismatch from the version in lerna.json (which maybe comes, in turn, from the version in package.json?).

I think the testapp being aware of lerna is lame, and I'd like to move in a direction where that's not the case. As a first step, I tried moving this repo's top-level package.json's lerna from devDependencies to dependencies, and that installed okay. The testapp globally installing is a patch to fix the real problem: postinstall is requiring that things in devDependencies are installed, but devDependencies aren't always installed. (e.g., if you do npm install foo, foo's devDependencies are not installed).

I'll commit to this PR and see if it works against quay's javascript-testapp:latest.

@spencerwilson-optimizely
Copy link
Contributor

Oh, and for the record, this PR is to satisfy SourceClear reporting vulns in this repo.

Copy link
Contributor

@spencerwilson-optimizely spencerwilson-optimizely left a comment

Choose a reason for hiding this comment

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

Cool, it works.

@aliabbasrizvi
Copy link
Contributor Author

@mikeng13 good to go?

@mikeproeng37
Copy link
Contributor

Yep! This is exactly what I was talking to you about yesterday. Thanks @spencerwilson-optimizely

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

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

lgtm

@aliabbasrizvi aliabbasrizvi merged commit a72082b into master Aug 30, 2018
@aliabbasrizvi aliabbasrizvi deleted the ali/update_library branch August 30, 2018 19:04
mikeproeng37 pushed a commit that referenced this pull request Sep 10, 2018
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