Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core.typed annotations #168

Closed
marcomorain opened this issue Feb 11, 2015 · 8 comments
Closed

Core.typed annotations #168

marcomorain opened this issue Feb 11, 2015 · 8 comments

Comments

@marcomorain
Copy link

We are using clj-time at CircleCI, and we have had to add core.typed annotations to the public functions that we use. Would you accept a PR to add core.typed annotations if I created one?

@michaelklishin
Copy link
Member

If this wouldn't affect the users who don't use core.typed in any way, I personally think it's worth considering.

@michaelklishin
Copy link
Member

Another concern is how much effort it may be to maintain them. Many of our contributors may not have a lot of experience with core.typed.

@marcomorain
Copy link
Author

Thanks Michael. It might make sense for me to publish a separate package
that contains just the type annotations for clj-time.

On Wed, Feb 11, 2015 at 10:33 AM, Michael Klishin notifications@github.com
wrote:

Another concern is how much effort it may be to maintain them. Many of our
contributors may not have a lot of experience with core.typed.


Reply to this email directly or view it on GitHub
#168 (comment).

@seancorfield
Copy link
Member

As someone who has used core.typed quite a bit -- and appreciates the value it can add in terms of bugs found etc, my main concern with adding type annotations to libraries - particularly ones that are widely used - is the massive pile of transient dependencies that core.typed drags in and the (small) overhead it adds (in terms of both source code readability and in performance and plain old bulk). And @michaelklishin is right about maintainability going forward.

I don't think the core.typed community has really settled on a good way to package annotations around libraries yet?

@apiology
Copy link

Hey @marcomorain, did you ever release that package? I'm also interested in clj-time types.

@marcomorain
Copy link
Author

@apiology I'm afraid not. This is our current set of type annotations:

(t/ann clj-time.core/date-time
       (t/IFn [t/Int -> DateTime]
              [t/Int t/Int -> DateTime]
              [t/Int t/Int t/Int -> DateTime]
              [t/Int t/Int t/Int t/Int -> DateTime]
              [t/Int t/Int t/Int t/Int t/Int -> DateTime]
              [t/Int t/Int t/Int t/Int t/Int t/Int  -> DateTime]
              [t/Int t/Int t/Int t/Int t/Int t/Int t/Int -> DateTime]))
(ann clj-time.core/latest
  (t/IFn [ReadableInstant ReadableInstant -> ReadableInstant]
         [(t/Coll ReadableInstant) -> ReadableInstant]))
(ann clj-time.core/now [-> DateTime])
(ann clj-time.format/parse (t/IFn
                            [String -> (Option DateTime)]
                            [(t/U DateTimeFormatter String) String -> (Option DateTime)]))
(ann clj-time.core/millis (t/IFn [t/Int -> Period]))
(ann clj-time.core/seconds (t/IFn [t/Int -> Seconds]))
(ann clj-time.core/minutes (t/IFn [t/Int -> Minutes]))
(ann clj-time.core/hours (t/IFn [t/Int -> Hours]))
(ann clj-time.core/days [Number -> Days])

(ann clj-time.core/in-millis (t/IFn [ReadablePeriod -> t/AnyInteger]))

(ann clj-time.format/formatter (t/IFn [String -> DateTimeFormatter]
                                   [String DateTimeZone -> DateTimeFormatter]
                                   [DateTimeZone String * -> DateTimeFormatter]))

(ann clj-time.coerce/to-long (t/IFn [nil -> nil]
                                 [java.util.Date -> Long]
                                 [java.sql.Date -> Long]
                                 [java.sql.Date -> Long]
                                 [DateTime -> Long]
                                 [Integer -> Long]
                                 [String -> (t/U Long nil)]
                                 [java.sql.Timestamp -> Long]))
(ann clj-time.core/after? [DateTime DateTime -> Boolean])
(ann clj-time.core/ago [ReadablePeriod -> DateTime])
(ann clj-time.core/before? [DateTime DateTime -> Boolean])
(ann clj-time.core/from-now [ReadablePeriod -> DateTime])
(ann clj-time.core/plus [DateTime ReadablePeriod -> DateTime])
(ann clj-time.core/minus [DateTime ReadablePeriod -> DateTime])

@marcomorain
Copy link
Author

And we have a bunch of not-nil-returns:

(t/non-nil-return org.joda.time.DateTimeZone/UTC :all)
(t/non-nil-return org.joda.time.Duration/standardSeconds :all)
(t/non-nil-return org.joda.time.Duration/standardMinutes :all)
(t/non-nil-return org.joda.time.Duration/standardHours :all)

;; All the different implementations of ReadablePeriod
(t/non-nil-return org.joda.time.Period/plus :all)
(t/non-nil-return org.joda.time.Period/minus :all)
(t/non-nil-return org.joda.time.Years/plus :all)
(t/non-nil-return org.joda.time.Years/minus :all)
(t/non-nil-return org.joda.time.Months/plus :all)
(t/non-nil-return org.joda.time.Months/minus :all)
(t/non-nil-return org.joda.time.Weeks/plus :all)
(t/non-nil-return org.joda.time.Weeks/minus :all)
(t/non-nil-return org.joda.time.Days/plus :all)
(t/non-nil-return org.joda.time.Days/minus :all)
(t/non-nil-return org.joda.time.Hours/plus :all)
(t/non-nil-return org.joda.time.Hours/minus :all)
(t/non-nil-return org.joda.time.Minutes/plus :all)
(t/non-nil-return org.joda.time.Minutes/minus :all)
(t/non-nil-return org.joda.time.Seconds/plus :all)
(t/non-nil-return org.joda.time.Seconds/minus :all)

@apiology
Copy link

@marcomorain Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants