Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

Replace npm shrinkwrap with yarn.lock #1278

Merged
merged 9 commits into from
Nov 23, 2016
Merged

Replace npm shrinkwrap with yarn.lock #1278

merged 9 commits into from
Nov 23, 2016

Conversation

hiddentao
Copy link
Contributor

@hiddentao hiddentao commented Oct 12, 2016

Did a fresh install using yarn and it took <1min. Way faster than npm install.

This replaces shrinkwrap with yarn.lock file so that we'll have more deterministic builds.

Info on yarn -> https://yarnpkg.com/

@luclu
Copy link
Contributor

luclu commented Oct 12, 2016

Do we need to adjust travis?

Copy link
Contributor

@luclu luclu left a comment

Choose a reason for hiding this comment

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

I guess we could include yarn into our CI as soon as we publish artifacts. Looks good!

@hiddentao
Copy link
Contributor Author

hiddentao commented Oct 13, 2016

I suppose we could use yarn within travis to install the deps. Thinking about it, we probably should do that since we no longer use shrinkwrap (which means npm install won't be consistent on travis).

EDIT: just saw your comment above. Yeah, once we build artifacts using yarn in CI becomes essential. Right now it's just a nice-to-have if we're not really building artifacts.

@hiddentao
Copy link
Contributor Author

This requires a bit more testing. After deleting node_modules and then running yarn and then trying gulp I got some errors regarding the absence of 7zip-bin-osx. This is an optionally dependency of 7zip-bin, which is a dep of electron-builder. I need to look into it more but it seems that Yarn might not be running through all the install steps NPM does.

@luclu
Copy link
Contributor

luclu commented Nov 17, 2016

@luclu
Copy link
Contributor

luclu commented Nov 21, 2016

Yay, starting from today travis natively supports yarn: travis-ci/travis-build#895

@luclu
Copy link
Contributor

luclu commented Nov 23, 2016

@hiddentao I fixed the 7zip OS-specific download by adding them as optionalDependencies to the package.json as described in https://yarnpkg.com/en/docs/dependency-types#toc-optionaldependencies.
I just see it's now also in the readme: https://github.com/electron-userland/electron-builder

Eventually updating yarn also does the trick as it now internal adds them.

@luclu
Copy link
Contributor

luclu commented Nov 23, 2016

This PR is ready to be merged.

It includes:

  • yarn as the new package manager
    • no more misunderstood npm-shrinkwrap.json
    • no problems anymore, just do yarn (new commands at yarn cheatsheet)
  • updated electron-builder as there were some problems with deprecated 7zip-modules
    • updated gulpfile according to minor changes
  • travis:
    • native support for yarn
    • enabled yarn cache
    • updated electron-builder dependencies
  • tested:
    • mac: all platforms
    • travis: linux
    • linux: linux, win

@evertonfraga
Copy link
Member

LGTM 👍

@lock
Copy link

lock bot commented Mar 31, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Mar 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants