Skip to content

Conversation

@zslabs
Copy link
Contributor

@zslabs zslabs commented Oct 14, 2016

As discussed in #124

README.md Outdated
}
}
return obj1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's in documentation, I think it would be better to use a library like lodash/merge or deepmerge for clarity - this is a lot of non-gulp-load-plugins code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I actually found extend to work really well -- I'll update it with that one, but if the others are more preferred I'm fine either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat 👍

@jackfranklin
Copy link
Owner

Looks good! Mega pedantic, but could you use Lodash as the library that provides extend? Just because that's most popular and understood I think.

Really appreciate your efforts here, be sure to add yourself to the contributors list in the package.json.

@zslabs
Copy link
Contributor Author

zslabs commented Oct 14, 2016

Sure thing, updated!

@jackfranklin
Copy link
Owner

(Sorry) but could you change the sentence before the code example to point to Lodash, not to extend? :)

@zslabs
Copy link
Contributor Author

zslabs commented Oct 14, 2016

Oof, updated

@jackfranklin jackfranklin merged commit 5f49d5f into jackfranklin:master Oct 16, 2016
@jackfranklin
Copy link
Owner

👍 thank you!

@zslabs zslabs deleted the patch-1 branch October 18, 2016 13:57
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