Skip to content

Conversation

@lini
Copy link
Contributor

@lini lini commented Aug 7, 2018

PR Checklist

What is the new behavior?

tns plugin create command added to CLI. The command will clone the plugin seed and execute the initial seed configuration script to prepare a new project for developing a NativeScript plugin.

- `tns plugin create` command
- add documentation
- add tests
@dtopuzov
Copy link
Contributor

run ci

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Can you please rebase your changes on the latest master, this will make your build green

const cwd = path.join(projectDir, "src");
try {
spinner.start();
await this.$childProcess.exec("npm i", { cwd: cwd });
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using childProcess.exec, you can use the $npm:
https://github.com/NativeScript/nativescript-cli/blob/master/lib/node-package-manager.ts#L18

For example:

await this.$npm.install(cwd, cwd);

}

let gitHubUsername = config.username;
if (!gitHubUsername) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you can extract part of this method to a separate methods, which will make the code easier to read, for example:

const gitHubUsername = await this.getGitHubUsernam(config.username);
const pluginNameSource = await this.getPluginNameSource(config.pluginName);


const params = `gitHubUsername=${gitHubUsername} pluginName=${pluginNameSource} initGit=y`;
// run postclone script manually and kill it if it takes more than 10 sec
const outputScript = (await this.$childProcess.exec(`node scripts/postclone ${params}`, { cwd: cwd, timeout: 10000 }));
Copy link
Contributor

Choose a reason for hiding this comment

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

this could cause issues in case any of the parameters has a space in it. I recommend using childProcess.spawn:

const pathToPostCloneScript = path.join("scripts", "postclone");
const params = [ pathToPostCloneScript, `gitHubUsername=${gitHubUsername}`, `pluginName=${pluginNameSource}`, "initGit=y"];
await this.$childProcess.spawnFromEvent(process.execPath, params, "close", { cwd, timeout: 10000 });

lib/options.ts Outdated
watch: { type: OptionType.Boolean, default: true },
background: { type: OptionType.String }
background: { type: OptionType.String },
username: {type: OptionType.String},
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please format this changes (Alt + Shift +F in Visual Studio Code)

lini added 2 commits August 22, 2018 09:36
- $npm.install replaces $childProcess.exec("npm i")
- extract prompts to separate functions
- $childProcess.spawnFromEvent replaces $childProcess.exec
- formatting
- update tests
@rosen-vladimirov
Copy link
Contributor

run ci

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Great contribution! Thank you!

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.

4 participants