-
Notifications
You must be signed in to change notification settings - Fork 18
Add Yarn option #105
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
base: master
Are you sure you want to change the base?
Add Yarn option #105
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,7 +81,7 @@ function installPristineApp(appName, options) { | |
| chdir(temp.pristinePath); | ||
|
|
||
| // Install a vanilla app and cd into it | ||
| let args = generateArgsForEmberNew(hasNodeModules, hasBowerComponents); | ||
| let args = generateArgsForEmberNew(hasNodeModules, hasBowerComponents, options); | ||
| let promise = runNew(appName, args) | ||
| .catch(handleResult) | ||
| .then(() => chdir(path.join(temp.pristinePath, appName))); | ||
|
|
@@ -91,7 +91,8 @@ function installPristineApp(appName, options) { | |
| hasNodeModules, | ||
| hasBowerComponents, | ||
| emberVersion: options.emberVersion || 'canary', | ||
| emberDataVersion: options.emberDataVersion || 'emberjs/data#master' | ||
| emberDataVersion: options.emberDataVersion || 'emberjs/data#master', | ||
| yarn: options.yarn || false | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am asking myself if it could make sense to not default to Maybe we could copy that behavior here? Fwiw, this could be a separate PR though... |
||
| }; | ||
|
|
||
| promise = promise | ||
|
|
@@ -111,9 +112,14 @@ function installPristineApp(appName, options) { | |
|
|
||
| // Generates the arguments to pass to `ember new`. Optionally skipping the | ||
| // npm and/or bower installation phases. | ||
| function generateArgsForEmberNew(skipNpm, skipBower) { | ||
| function generateArgsForEmberNew(skipNpm, skipBower, options) { | ||
| let extraOptions = []; | ||
|
|
||
| if (options.yarn) { | ||
| debug('using yarn'); | ||
| extraOptions.push('--yarn'); | ||
| } | ||
|
|
||
| if (skipNpm) { | ||
| debug('skipping npm'); | ||
| extraOptions.push('--skip-npm'); | ||
|
|
@@ -142,11 +148,19 @@ function nodeModulesSetup(options) { | |
| promise = promise | ||
| .then(() => { | ||
| debug('installing ember-disable-prototype-extensions'); | ||
| return runCommand('npm', 'install', 'ember-disable-prototype-extensions'); | ||
| if (options.yarn) { | ||
| return runCommand('yarn', 'add', 'ember-disable-prototype-extensions'); | ||
| } else { | ||
| return runCommand('npm', 'install', 'ember-disable-prototype-extensions'); | ||
| } | ||
| }) | ||
| .then(() => { | ||
| debug("installed ember-disable-prototype-extension"); | ||
| return runCommand('npm', 'install'); | ||
| if (options.yarn) { | ||
| return runCommand('yarn'); | ||
| } else { | ||
| return runCommand('npm', 'install'); | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following up on my comment https://github.com/tomdale/ember-cli-addon-tests/pull/105/files#r118477645 and I think also what @kellyselden was suggesting, if this addon knows which package manager to use, maybe we can abstract this and the Can be a separate PR as well, just wanted to mention my thoughts here... |
||
| }) | ||
| .then(() => debug('installed ember-data ' + emberDataVersion)) | ||
| .then(() => symlinkAddon(appName)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,15 +43,16 @@ describe('Acceptance | application', function() { | |
| promoteHtmlbars(); | ||
|
|
||
| return app.create('dummy', { | ||
| fixturesPath: 'tests' | ||
| fixturesPath: 'tests', | ||
| yarn: true | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps we should leave this test intact and make a new one that tests yarn. Then both our npm and yarn code paths get tested. |
||
| }); | ||
| }).then(function() { | ||
| process.chdir(previousCwd); | ||
| }).then(function() { | ||
| app.editPackageJSON(function(pkg) { | ||
| pkg.devDependencies['ember-cli-fastboot'] = process.env.npm_package_devDependencies_ember_cli_fastboot; | ||
| }); | ||
| return app.run('npm', 'install'); | ||
| return app.run('yarn', 'install'); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
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.
accidental double slash?