-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add 'wdm' to bundled CLI 2.x Gemfile #814
Conversation
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Poitrin! Great stuff 🚀 I've left only one minor comment/question :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Poitrin! LGTM and works fine as well 🎩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This update looks nice but I am slightly confised about 3 of the steps on Windows that are listed. What exactly do these steps mean?
|
@AaronGabriel147 they are to test that this change is working correctly using the development version of the code. As a user you don't need to worry about them. |
WHY are these changes introduced?
Fixes #780
WHAT is this pull request doing?
shopify-cli
is using the gemlisten
, which callsrequire 'wdm'
, without having it mentioned in itsGemfile
(→ guard/listen#553).To solve this, we install
wdm
via the bundled CLI's Gemfile.How to test your changes?
Windows
cli
repo andcd
into ityarn install
andyarn build
cd
into a valid theme directorygem which wdm
returnsCan't find Ruby library file or shared library wdm
node ../cli/packages/cli-main/bin/dev.js theme dev
wdm
should appear anymoreC:\Users\<YOUR USER NAME>\AppData\Local\shopify-cli-nodejs\Cache\vendor\ruby-cli\<2.x.x>\Gemfile
and make sure thatgem 'wdm'
is mentioned theremacOS
/Users/<YOUR USER NAME>/Library/Caches/shopify-cli-nodejs/vendor/ruby-cli/<2.x.x>/Gemfile
and make sure thatgem 'wdm'
is not mentioned therePost-release steps
Measuring impact
How do we know this change was effective? Please choose one:
Checklist
dev
ordeploy
have been reflected in the internal flowchart.