GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.17k stars 155 forks source link

Changes the use of Intervals/Timestamps as long into Kotlin Duration/Instant #553

Closed Daeda88 closed 4 months ago

Daeda88 commented 4 months ago

Fixes a bit of a pet peeve to me: When dealing with Intervals and Timestamps, on Kotlin we should be using Kotlin Duration and Kotlin DateTime Instant instead of using Longs.

My reasoning behind this:

nbransby commented 4 months ago

Durations aren't designed to be used as timestamps, we should probably be using https://kotlinlang.org/api/kotlinx-datetime/kotlinx-datetime/kotlinx.datetime/-instant/

Daeda88 commented 4 months ago

Durations aren't designed to be used as timestamps, we should probably be using https://kotlinlang.org/api/kotlinx-datetime/kotlinx-datetime/kotlinx.datetime/-instant/

Right, though I guess the ticket is also a bit of a misnomer as most of these are intervals not timestamps, for which duration is the right class.

Firebase.storage.setMaxOperationRetryTimeMillis(10000) -> Firebae.storage.setMaxOperationRetryTime(10.seconds)

to me is the main reason for changing this (well that and is confusion. In the code on master, that 10000 was not divided by 1000 so it would be set to 10000 seconds on iOS)

nbransby commented 4 months ago

Agree the timeouts should be Durations

Daeda88 commented 4 months ago

Ok, there was one place where Instant made more sense (in the config module). Moved to it there