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

Use the current package.json name value for ember init. #491

Merged
merged 1 commit into from
Apr 29, 2014

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Apr 28, 2014

Closes #395.


Please note I did not add tests for this, as this code path is completely refactored (and tested) in #466.

@@ -18,7 +18,8 @@ module.exports = new Command({

run: function(environment, options) {
var cwd = process.cwd();
var rawName = path.basename(cwd);
var pkg = require(cwd + '/package');
Copy link
Contributor

Choose a reason for hiding this comment

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

this will fail if no package is present? seems incorrect

@stefanpenner
Copy link
Contributor

@rwjblue
Copy link
Member Author

rwjblue commented Apr 28, 2014

Just updated to use Project model. Gonna try to bang out some tests next, but is this the path you were thinking of?

@rwjblue
Copy link
Member Author

rwjblue commented Apr 28, 2014

Now with tests.

@stefanpenner
Copy link
Contributor

can you just put project in the outer environment object: https://github.com/stefanpenner/ember-cli/blob/master/lib/cli/index.js#L37

and inject it here: https://github.com/stefanpenner/ember-cli/blob/master/lib/cli/cli.js#L26 on commands?

This may seem a bit strange, but it keeps knowledge of how to get the project instance isolated from commands, which is more aligned with the command refactor.

@@ -9,7 +9,7 @@ module.exports = new Task({
// Options: Boolean dryRun
run: function(ui, options) {
var cwd = process.cwd();
var rawName = path.basename(cwd);
var rawName = options.rawName || path.basename(cwd);
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like we never want to fallback to path.basename instead always requiring the correct inputs

Copy link
Member Author

Choose a reason for hiding this comment

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

Just refactored as you suggested earlier. I've still got a couple failures I'm working through though.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok, thanks! Just reviewing before i got to sleep

Copy link
Contributor

Choose a reason for hiding this comment

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

this still seems strange

Copy link
Member Author

Choose a reason for hiding this comment

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

Remove the fallback.

Refactor `CLI` to inject the current Project into each command.
@rwjblue
Copy link
Member Author

rwjblue commented Apr 29, 2014

Updated with latest round of formatting tweaks. Please let me know if there is anything else needed.

@stefanpenner
Copy link
Contributor

looks good

rwjblue added a commit that referenced this pull request Apr 29, 2014
Use the current package.json name value for `ember init`.
@rwjblue rwjblue merged commit c49e981 into ember-cli:master Apr 29, 2014
@rwjblue rwjblue deleted the use-project-package-for-init branch April 29, 2014 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ember init does not respect app-name
2 participants