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

Adds yarn, updates travis to install using yarn #252

Merged
merged 1 commit into from Oct 17, 2016

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Oct 17, 2016

Yarn does not yet jive with bower, when it does this build step will be removed: yarnpkg/yarn#896

Added gulp-imagemin optional dependencies

closes #245

@chriskacerguis @himdel

Yarn does not yet jive with bower, when it does this build step will be removed: yarnpkg/yarn#896
@miq-bot
Copy link
Member

miq-bot commented Oct 17, 2016

Checked commit AllenBW@7d5d0d3 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. ⭐

@chriskacerguis chriskacerguis merged commit 9ea93b9 into ManageIQ:master Oct 17, 2016
@chriskacerguis chriskacerguis self-assigned this Oct 17, 2016
@chriskacerguis chriskacerguis added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 17, 2016
@AparnaKarve
Copy link
Contributor

AparnaKarve commented Oct 17, 2016

@chriskacerguis Looks like this is a done deal.
So going forward would we be using yarn instead of npm?

@chriskacerguis
Copy link
Contributor

Hi @AparnaKarve yes, that is correct (the readme is updated).

@AllenBW
Copy link
Member Author

AllenBW commented Oct 17, 2016

@AparnaKarve you're correct, yarn or yarn install will now be the way to setup dependencies

We aren't exactly using yarn instead npm. npm is still very much involved in the app, you could think of yarn as an package.json manager.

@AllenBW AllenBW deleted the enhancement/245-add-yarm branch October 17, 2016 22:13
@himdel
Copy link
Contributor

himdel commented Oct 18, 2016

@AllenBW I'm seeing a error npm-shrinkwrap.json found. This will not be updated or respected. See [TODO] for more information. - we should probably reflect what's in shrinkwrap to yarn.lock and remove that shrinkwrap...

And another thing, should we remove scripts.install & scripts.postinstall from package.json now? Feels like we shouldn't be using npm-specific scripts if we're not using npm..

@AllenBW
Copy link
Member Author

AllenBW commented Oct 18, 2016

@himdel Thanks for the feedback!! So first point, do we need the shrinkwrap anymore? (After removing the file and running yarn install again)gulp templatecache operates as expected, updating the dependencies likely resolved this isssssssssssuuuuueeeeeeee. 🌮

☝️ so you mind if i axe the file in question?

Second point, SO a cool thing yarn does is yarn run foo where foo can be the name of any script we have in our package.json.... the bummer is right now it doesn't quite work (see yarnpkg/yarn#1156) , but IDEALLY I would like to keep the scripts there, and references them in the future rather than rewrite them all the places they are required

@himdel
Copy link
Contributor

himdel commented Oct 18, 2016

Right.. looking at the issue that caused us to have npm-shrinkwrap in the first place (gulp-community/gulp-header#37), it's resolved and probably only affected gulp-header 1.8.3, so 1.8.8 should be fine.

So feel free to completely and utterly destroy npm-shrinkwrap.json ;).

And if the scripts are planned to be supported, then sure, let's keep them :)

AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this pull request Oct 18, 2016
` error npm-shrinkwrap.json found. This will not be updated or respected. See [TODO] for more information.`

As per ManageIQ#252
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to yarn for packages?
5 participants