search

andrewmcveigh / cljs-time

A clj-time inspired date library for clojurescript.
342 stars 57 forks source link

Consider using cljs-time.core/equal? as well as cljs-time.core/= #54

Closed markwoodhall closed 8 years ago

markwoodhall commented 8 years ago

Hi, thanks for creating cljs-time, excellent work. :+1:

I wanted to ask if there is a reason why you can't consider using cljs-time.core/equal? as well as cljs-time.core/=. This would bring the API inline with what clj-time uses.

I'm currently making use of reader conditionals to use either clj-time.core/equal? or cljs-time.core/=, it would be cool to not have to do this.

Thanks!

andrewmcveigh commented 8 years ago

Hi, thanks for the report.

No reason, other than clj-time.core/equal? only went into clj-time a few months ago and I haven't caught up yet! I've been lagging a bit behind the recent API movements due to being a bit busy :-/

I'd happily accept a pull-request if you fancy it, otherwise I'll get it in the next release.

markwoodhall commented 8 years ago

Ah, I didn't realise it was relatively new to clj-time. I'll happily create a pull request. Are we just talking aliasing the internal cljs-time = function as equal??

andrewmcveigh commented 8 years ago

I think I'd copy more-or-less the way clj-time/JodaTime deal with it, as cljs-time.core/= (actually this is an alias to cljs-time.internal.core/=) does a bit too much.

clj-time/equal? is defined as a protocol function, so I think it'd be best to follow that.

 (equal? [this ^ReadableInstant that] (.isEqual this that))

https://github.com/clj-time/clj-time/blob/master/src/clj_time/core.clj#L149