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

Allow ncp filter option #12

Merged
merged 4 commits into from
Sep 15, 2018
Merged

Allow ncp filter option #12

merged 4 commits into from
Sep 15, 2018

Conversation

SirMomster
Copy link
Contributor

Small changes that would allow usage of the ncp filter option when using copy-node-modules programatically.

@arloliu arloliu requested a review from ezze September 14, 2018 15:36
index.js Outdated
@@ -140,7 +144,7 @@ function copyNodeModules(srcDir, dstDir, opts, callback)

if (!callback)
{
g_opts = {srcDir: srcDir, dstDir: dstDir, devDependencies: false};
g_opts = {srcDir: srcDir, dstDir: dstDir, devDependencies: false, filter: null};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably, you don't need to set filter option to null by default as it's already undefined.

Copy link
Collaborator

@ezze ezze left a comment

Choose a reason for hiding this comment

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

@SirMomster, thank you for your pull request, nice addition!

But I have some proposals:

  1. ncp also supports filter option through CLI. It would be nice to have this option in CLI of copy-node-modules too. Can you add it?

  2. filter option should be described in README.md for both programmatic and command line usage to let other people know about it. It would be also nice to mention it in JSDoc of copyNodeModules function.

Thanks.

@SirMomster
Copy link
Contributor Author

SirMomster commented Sep 15, 2018

@ezze Thanks, I'm glad that I can help, this is my first pull request and your response broke the ice for me.

I added support for the CLI it allows Regular Expressions now. But in ncp they match the folder path before matching files. I thought this was important to know so I added it to the readme.md.

I totally forgot to do the documentation, sorry for that. But I did it now, I added an example function for a filter method, the JSDoc option and documentation for the CLI.

If something is wrong let me know and I will fix it.

Fyi. I use your module for a Lerna, typescript project with AWS Lambda, copying node_modules that where "linked" included there dependencies, which I at first removed afterwards but this took a lot of time. Which resulted in this pull request. Thanks for the plugin, it is a great addition to my setup! Saving me about 30-60 mins a day.

@arloliu arloliu merged commit a8ac929 into arloliu:master Sep 15, 2018
@arloliu
Copy link
Owner

arloliu commented Sep 15, 2018

Thanks for your help, I'll merge this pull and publish a new version.

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