Skip to content

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

Merged
merged 1 commit into from
Jul 14, 2014
Merged

Create project command #5

merged 1 commit into from
Jul 14, 2014

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Jul 11, 2014

No description provided.


$injector.requireCommand("create", "./commands/create-project-command");

$injector.require("nodePackageManager", "./node-package-manager");
Copy link
Contributor

Choose a reason for hiding this comment

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

just call it npm

Copy link
Contributor Author

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 :)

Copy link
Contributor

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.

@Fatme
Copy link
Contributor Author

Fatme commented Jul 11, 2014

@tailsu I addressed all your comments. I suggest to extract common blocks in another PR. Agree?

@tailsu
Copy link
Contributor

tailsu commented Jul 11, 2014

Better do it now as we keep postponing these clean-ups and it slows us down in the long term.

@Fatme
Copy link
Contributor Author

Fatme commented Jul 11, 2014

@tailsu

1 similar comment
@Fatme
Copy link
Contributor Author

Fatme commented Jul 11, 2014

@tailsu

"p" : "path"
};

var defaultProfileDir = path.join(process.env.USERPROFILE || osenv.home() || process.env.HOMEPATH, ".nativescript-cli");
Copy link
Contributor

Choose a reason for hiding this comment

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

use osenv.home() here

@tailsu
Copy link
Contributor

tailsu commented Jul 11, 2014

👍

Fatme added a commit that referenced this pull request Jul 14, 2014
@Fatme Fatme merged commit 0646ab3 into master Jul 14, 2014
@Fatme Fatme deleted the fatme/create-project-command branch July 14, 2014 06:28
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);
Copy link

Choose a reason for hiding this comment

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

We are missing some %ses here.

@Fatme
Copy link
Contributor Author

Fatme commented Jul 14, 2014

goot catch :) I'll fix it.

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.

3 participants