search

weavejester / medley

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

Add mapply, remove-keys, and remove-vals. #6

Closed ToBeReplaced closed 10 years ago

ToBeReplaced commented 10 years ago

Adding mapply, remove-keys, and remove-vals as per: https://github.com/weavejester/medley/issues/3 https://github.com/weavejester/medley/issues/4

weavejester commented 10 years ago

Thanks for the commits, but there are a couple of small things to fix before I can merge.

  1. Could you remove commit 9cc3482. It doesn't relate to the pull request, and patches shouldn't really increment version numbers.
  2. The summary line (i.e. the first line) of the commit message of d51cf36 is a bit too long. Generally it's a good idea to keep this under 70 characters, so that it's not cut off on log views.
  3. In general it's a good idea to have a pull request per issue, but this isn't as important as 1 and 2, because it's a relatively small change.
ToBeReplaced commented 10 years ago

Understood -- new commits are modified as per 1) and 2). I should not have messed with the version number or dependencies. I was just cutting corners sending one PR since your say on the Issues seemed pretty final. In the future on your projects I'll send separate PRs per issue, or if you'd like, patches instead of PRs.

ToBeReplaced commented 10 years ago

Yeah... I should have used complement. Nice catch, and thanks for the patience. You could always factor out the remaining reduce-kv calls as well.