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

Fixes integrity check for --production flag #3067

Merged
merged 3 commits into from
Apr 7, 2017
Merged

Fixes integrity check for --production flag #3067

merged 3 commits into from
Apr 7, 2017

Conversation

bestander
Copy link
Member

Summary

Fixes issue revealed in #2819

Test plan

Added test

@bestander bestander requested a review from arcanis April 7, 2017 13:02
@bestander bestander merged commit 13c8bca into yarnpkg:master Apr 7, 2017
arcanis pushed a commit to arcanis/yarn that referenced this pull request Apr 7, 2017
@bestander
Fixes integrity check for --production flag (yarnpkg#3067)
63f4f2c 
* fixed integrity check when running with --production

* added test

* removed unused var
bestander pushed a commit that referenced this pull request Apr 7, 2017
@arcanis @bestander
0.23.1 cherry-picking (#3071)
3320806 
* Checks that the webpack builds are working properly (#3064)

* Fix missing subdeps in production install when those are present in devDependencies list (#2921)

* Test for install skipping subdependencies when those are named in root devDependencies

* Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819

* Unconditionally mark packages as ignored, hoister now fully corrects transitive uses

* replaced deprecated asserts (#3069)

* fixing lint (#3070)

* Fixes integrity check for --production flag (#3067)

* fixed integrity check when running with --production

* added test

* removed unused var

* Remove the dependency on the "rc" module (#3063)

* Removes dependency on the "rc" module

* Removers shebang-loader, not used anymore

* Fixes flow errors

* Fixes tests on Windows
@DanBuild DanBuild mentioned this pull request Apr 13, 2017
Closed
@SimenB
Copy link
Contributor

SimenB commented Apr 13, 2017

Interesting that the repro from #2819 still logs a warning in 0.23.2. What is its purpose?

yarn init -y && yarn add sequelize-cli && yarn add gulp --dev && yarn --prod && yarn check --verify-tree --prod

I find the 2 last commands interesting

yarn install v0.23.2
[1/4] 🔍  Resolving packages...
warning Integrity check: Flags don't match
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
✨  Done in 2.09s.
yarn check v0.23.2
success Folder in sync.
✨  Done in 1.01s.

Why complain about "Flags don't match" when it's not wrong? What actions can I as a user do? And what does it really mean?

@bestander
Copy link
Member Author

Good point, @SimenB.

So let me explain:

yarn init -y && yarn add sequelize-cli

adds sequelize-cli to dependencies in non production mode

yarn add gulp --dev

adds gulp to dev dependencies in non production mode

yarn --prod

installs dependencies in production mode.
Because previous install (add gulp) was done in non production mode the integrity check does not pass and Yarn proceeds with doing a full installation.

But I think you are right, there should not be a warning, I added it for information purposes when running yarn check --integrity.
I think integrity checker should not report, check.js command should do that.

@bestander bestander mentioned this pull request Apr 21, 2017
Closed
@SimenB
Copy link
Contributor

SimenB commented Apr 21, 2017

Ah, OK. Thanks for the explanation 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcanis arcanis arcanis approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@bestander @SimenB @arcanis