taoensso / faraday

Amazon DynamoDB client for Clojure
https://www.taoensso.com/faraday
Eclipse Public License 1.0
238 stars 84 forks source link

Issue with update-map being deprecated on update-item #83

Closed ricardojmendez closed 8 years ago

ricardojmendez commented 8 years ago

There's a warning that update-map has been deprecated on update-item in favor of :cond-expr. Two issues:

Given that we're already doing breaking changes for 1.9, isn't this a good time to remove it altogether?

ptaoussanis commented 8 years ago

Hi Ricardo, thanks for catching this.

As a general rule, I'd like to try keep any breaking changes as small as possible.

In this case, what if:

  1. We make update-map optional (a breaking change) but
  2. Retain support for update-map with the (corrected) warning

This way folks adopting to the break will be forced to update only the arg format, not the update logic.

Thanks again for all your work on Faraday btw, looking forward to seeing the results :-)

ricardojmendez commented 8 years ago

This way folks adopting to the break will be forced to update only the arg format, not the update logic.

Sounds good, I'll move it into the map with the rest of the options and update the comments.

Thanks again for all your work on Faraday btw, looking forward to seeing the results :-)

My pleasure. I'll soon need to get back to the actual project, so I'll start making changes as I go instead of in a big push, but it was a good way to get acquainted with the library.

I'll wrap this issue, then open up a request for that alpha release you mentioned. I'll probably write a brief tutorial now that's it's all fresh in my mind, and it would be useful to have an official alpha/beta release out.

Cheers!

ricardojmendez commented 8 years ago

Done.