Skip to content

normalize copy plugin. Fixes #166 #167

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

azhang
Copy link

@azhang azhang commented Feb 19, 2014

No description provided.

function normalize(conf, list){
return conf == list[0]
? conf.name
: require('path').basename(conf.path());
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks weird; why not just use basename defined at L14?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure why I did that. Removed L14.

Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to not require() inside a function, since it's sync ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yields shouldn't matter, as path has been required multiple times areadly in this file

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yeah, i know require()'s are cached :) just looks weird IMHO haha

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah looks very weird i agree

Copy link
Author

Choose a reason for hiding this comment

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

Okay I’ve moved it.
-- 
Aaron Zhang

On March 10, 2014 at 3:28:49 PM, Stephen Mathieson (notifications@github.com) wrote:

In lib/plugins/copy.js:

@@ -62,3 +63,19 @@ module.exports = function(type, dest, opts){
batch.end(done);
};
};
+
+
+/**

  • * Normalize conf name.
  • * @param {Object} conf
  • * @param {Array} list
  • * @return {String}
  • * @api private
  • */
    +
    +function normalize(conf, list){
  • return conf == list[0]
  • ? conf.name
  • : require('path').basename(conf.path());
    yeah looks very weird i agree


Reply to this email directly or view it on GitHub.

aaronz8 added 2 commits March 10, 2014 15:17
moved require('path').basename outside of the function
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants