search

weavejester / medley

A lightweight library of useful Clojure functions
Eclipse Public License 1.0
865 stars 66 forks source link

Changes in deep-merge fn #41

Closed velios closed 4 years ago

velios commented 4 years ago

Proposal of changes to avoid behaviour

user=> (deep-merge {:a 1} nil)
nil

and make it like

user=> (merge {:a 1} nil)
{:a 1}

Code take from discussion https://gist.github.com/danielpcox/c70a8aa2c36766200a95

weavejester commented 4 years ago

Thanks for the commit. Can you ensure the commit message matches the contributing guidelines, and it's usually not a good idea to remove code, so the additional arities you've taken away need to be added back in.

I'll also take a look at this myself to see if there's a less impactful fix.

weavejester commented 4 years ago

Okay, I believe I have a fix that improves performance a little as well. It doesn't share any code in common with this patch (except the test), so I'll commit separately.

velios commented 4 years ago

Thank you for the answer. This is my first github PR. I'll try better next time.

weavejester commented 4 years ago

For a first attempt it was very good. I'm just picky about commit message syntax, and in the case of Medley, picky about performance.

DmitrySuchkov-Arrival commented 4 years ago

@weavejester When do you plan to release this hotfix?