-
-
Notifications
You must be signed in to change notification settings - Fork 197
Create project command #5
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
Conversation
|
||
$injector.requireCommand("create", "./commands/create-project-command"); | ||
|
||
$injector.require("nodePackageManager", "./node-package-manager"); |
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.
just call it npm
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.
I tried it but the module that is in package.json
is also named npm
:)
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.
That shouldn't be a problem, I think. The package is resolved using require('npm') and our class is resolved using yok, so there shouldn't be a collision.
@tailsu I addressed all your comments. I suggest to extract common blocks in another PR. Agree? |
Better do it now as we keep postponing these clean-ups and it slows us down in the long term. |
1 similar comment
"p" : "path" | ||
}; | ||
|
||
var defaultProfileDir = path.join(process.env.USERPROFILE || osenv.home() || process.env.HOMEPATH, ".nativescript-cli"); |
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.
use osenv.home()
here
👍 |
this.$errors.fail("Path already exists and is not empty %s", projectDir); | ||
} | ||
|
||
this.$logger.trace("Creating a new NativeScript project with name %s and id at location", projectName, projectId, projectDir); |
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.
We are missing some %s
es here.
goot catch :) I'll fix it. |
…ys-info Add more methods to sys info
No description provided.