Repository navigation
Js Support - #250
Js Support#250
Conversation
This reverts commit 0c78ac8.
wellingtoncosta
left a comment
wellingtoncosta
left a comment
There was a problem hiding this comment.
Left some comments. LGTM overall.
wellingtoncosta
left a comment
wellingtoncosta
left a comment
There was a problem hiding this comment.
I tested locally and it looks good to me.
DanielSouzaBertoldi
left a comment
DanielSouzaBertoldi
left a comment
There was a problem hiding this comment.
Good direction overall, most of the comments are really minor stuff, but there are some of them I think it's really important that we address before we merge. Great work! 🚀
| lifecycle.whileAttached { | ||
| try { | ||
| syncService.runSynchronization(conference = conference) | ||
| syncService.forceSynchronize(conference) |
There was a problem hiding this comment.
This seems like dead code to me 🤔
While reading the runSynchronization code, I noticed it suspends inside a coroutineScope { launch { while(isActive) } } and it just never returns while the scope is alive.
There was a problem hiding this comment.
Also, digging a bit deeper, I believe that we don't even need this forced synchronization.
runSynchronization instantiates the lastSessionizeSyncThisLoop with the current time - 3 hours:
var lastSessionizeSyncThisLoop: Instant = dateTimeService.now().minus(3, DateTimeUnit.HOUR)Which a bit down below timeToSync checks if lastSessionizeSyncThisLoop is equals or more than 15 minutes ago:
// simplified
val timeToSync = lastSessionizeSyncThisLoop <= dateTimeService.now().minus(15, DateTimeUnit.MINUTE)Which means that runSynchronization will always trigger a sync when it first runs.
What do you think? Is there a corner case of some sort I'm not aware of?
There was a problem hiding this comment.
I think there was a corner case but I don't remember at this time. Let me play around with it and see if there's an issue.
DanielSouzaBertoldi
left a comment
DanielSouzaBertoldi
left a comment
There was a problem hiding this comment.
Good direction overall, most of the comments are really minor stuff, but there are some of them I think it's really important that we address before we merge. Great work! 🚀
Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…/session/SessionDetailViewModel.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…/session/SessionDayViewModel.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…/session/BaseSessionListViewModel.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…mposeView.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…ce/impl/DefaultApiDataSource.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…ce/impl/DefaultApiDataSource.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…ce/impl/DefaultApiDataSource.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
…ce/impl/DefaultApiDataSource.kt Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
Co-authored-by: Daniel Bertoldi <dsdanielbertoldi@gmail.com>
Web Support
This PR adds Web support for Droidcon.
How to run:
./gradlew web:jsBrowserDevelopmentRunTo test local times, you need to set
testNotificationTimes = trueinshared/src/commonMain/kotlin/co/touchlab/droidcon/domain/service/impl/DefaultSyncService.ktScreenshots
I have to fix these