search

unclechu / node-deep-extend

Recursive extend module
MIT License
202 stars 53 forks source link

Added workaround for Moment instances #31

Closed neemzy closed 8 years ago

neemzy commented 8 years ago

Hi,

I noticed the lib is unable to deal with Moment instances. I therefore added a workaround to avoid processing them.

I'd understand if you don't want to merge this, as it is undesirable to handle workarounds for miscellaneous libraries, but you can get rid of it in your next major version (where you said you'd implement custom strategies, allowing such stuff to happen in userland) and allow people to use your library with Moment, the most popular date/time management library in the JS ecosystem, in the meantime.

Best regards!

unclechu commented 8 years ago

@neemzy sorry, but I'm not going to merge it because of:

  1. It's unwanted additional dependency;
  2. It's special case, we can't make this library be fat adding logic for every special case, it supposed to be small;
  3. Look at your changes, you mixed up spaces and tabs, there's .editorconfig file inside repo to avoid this (install editorconfig plugin to your code editor or IDE to prevent this mess in the future).

In next major release I planned to implement API for custom strategies for special cases like this one.

neemzy commented 8 years ago
  1. The additional dependency is specified as a devDependency; it will only be installed if you run npm install on this sole repository, in order to work on it and run tests, where this is actually needed. You'll notice I avoided making use of Moment in the library code precisely in that aim.
  2. I already adressed that concern. This fix was meant to be temporary and disappear when rendered obsolete by the next major version. Of course, I respect your point of view as I explained in the PR comment, but I'm beginning to wonder if you actually read it ;)
  3. (and second comment) No need to go all lofty on me just because I skipped your project's indentation style. As for the test, it does guarantee the library works with Moment instances by preserving its methods.

If I may, you might want to be extra careful when dealing with contributors, even if you don't like their work and aren't going to merge it, which you are absolutely free to do without using that kind of tone. Will not contribute again. Cheers! :)