search

funcool / buddy-hashers

Collection of password hashers.
https://funcool.github.io/buddy-hashers/latest/
Apache License 2.0
75 stars 16 forks source link

Breaking changes #10

Closed donbonifacio closed 8 years ago

donbonifacio commented 8 years ago

Hello,

I'd like to point out that this new 0.11.0 release has breaking changes, and I wasn't expecting that (do you use semver?) We were using :algorithm and now the param is :alg. This means that just updating the dep broke our suite.

You also removed md5, witch I understand. However this is troublesome for us. We use a recommended hasher for production, and se use md5 on dev machines just for speed. We will change our code for this situation, because the speed penalty is hurting us.

Overal, I'd just like to say that this is the 2nd time that an update broke our suite, and this is uncommon on clojure world.

niwinz commented 8 years ago

Hi @donbonifacio

The md5 and sha256 hasher was deprecated in 0.9.0 and is notified properly in the changelog, the same for the :algorithm parameter. And after two versions having marked that as deprecated, I have removed them. Having two versions for "deprecation => complete removing" path, I think is pretty conservative and allows gradually migrate old things in favor of new.

Also, the documentation says properly the state of the "Maturity" of the api saying that it can be some api breakage.

I know, it is maybe uncommon in clojure world, but I think is clearly warned that it can happens until the api is finally stabilized and 1.x.y version is released (in any case I don't expect much more api breakage, I'm pretty happy with the current approach).

If you are using the md5 hasher in development you could have notified me about that and I will rethought about finally removing the md5 hasher. Now having md5 removed, we can think in something like :dummy hasher that just serves for development purposes and help you and similar use cases for deal with that situations. Any onions about that?

donbonifacio commented 8 years ago

Hey @niwinz

I do understand all you've done (honestly, I didn't know about the deprecation warning).

The dummy might be a nice idea. Changing from md5 to strict (just one I tried), made our test suite go from 0.2s to 25s. :) We ended up coding a bypass when on dev, so that one is covered.

I just wanted to make a case for not breaking code with minor releases. I do reckon that when I had some trouble I got all the needed info from the changelog. :)

niwinz commented 8 years ago

I'll make the best effort for not breaking anymore (or more aggresively warning about that). In any case as I said previously, I'm pretty happy with current api so I don't expect any change.

But, I'm still recommend read the changelog on every update, I'm documenting every change there. I will give more thought about :dummy hasher just for testing and surelly will release it in the next version.

Thanks!

donbonifacio commented 8 years ago

:+1: