search

taoensso / tower

i18n & L10n library for Clojure/Script
https://www.taoensso.com/tower
Eclipse Public License 1.0
278 stars 24 forks source link

fix clojurescript example #64

Closed martinklepsch closed 9 years ago

martinklepsch commented 9 years ago

I wasn't able to get the example running without the modifications made here. :require statement has been simplified for CLJS > 2755 [1]. Let me know if I should add a comment about that.

1: https://groups.google.com/forum/#!topic/clojure/FoiqNV5nunQ

ptaoussanis commented 9 years ago

Hi Martin, thanks for getting in touch.

Could you explain what problem you were running into exactly? Both lines do look correct to me as they were. Are you running the latest version ([com.taoensso/tower "3.1.0-beta3"])?

martinklepsch commented 9 years ago

Hey Peter, sorry for this short-sighted PR. You're right of course. I used the stable version where it was still dict-compile without an *. Out of curiousity: what does the asterisk mean here?

I was somehow assuming documentation in the README refers to stable. Commenting all code snippets with a version number sounds clunky but maybe that'd be an option to avoid future confusion?

Sorry again, should have checked the changelog instead of mindlessly copying code from other people. :)

Thanks for Tower and all the other Clojure libs you've written!

ptaoussanis commented 9 years ago

Hey Martin, no problem at all.

Out of curiousity: what does the asterisk mean here?

Asterisk's there just to distinguish between the macro form (asterisk) and fn form (no asterisk). This is a bit of historical cruft.

I was somehow assuming documentation in the README refers to stable. Commenting all code snippets with a version number sounds clunky but maybe that'd be an option to avoid future confusion?

That's a totally reasonable assumption - it should be documented. Actually, I've got an entirely new translations lib which I'd like to publish when I get some time - still need to decide if I can back port to Tower or if it'd be better to release separately. Was planning to clean up all the docs then, but haven't had an opportunity yet.

Sorry again, should have checked the changelog instead of mindlessly copying code from other people. :)

Really no need to apologise - I'm sorry the README isn't clearer right now. Anyway, happy you've got it working now.

Thanks for Tower and all the other Clojure libs you've written!

You're very welcome, thanks for saying so!

martinklepsch commented 9 years ago

Would you like me to adapt the PR to add a small not about the version of Tower for that particular snippet?

Actually, I've got an entirely new translations lib which I'd like to publish when I get some time

Curious to see what you came up with. If you're looking for testers feel free to ping me.

ptaoussanis commented 9 years ago

Would you like me to adapt the PR to add a small not about the version of Tower for that particular snippet?

Sure, thanks!

Curious to see what you came up with. If you're looking for testers feel free to ping me.

Thanks. Already have the lib in prod, just need to find some time to publish and/or backport. Thinking I might include the new API along side the old to help ease gradual migrations.

martinklepsch commented 9 years ago

Updated the PR.

Thanks :)