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.
| } | ||
| } | ||
| } | ||
| /* |
| ), | ||
| ) | ||
|
|
||
| return initKoin( |
There was a problem hiding this comment.
Philosophical discussion: should all dependencies be a single?
There was a problem hiding this comment.
That's a fair question. Some should definitely be singles such as the Database or Settings, ResourceReader could be anything. I don't think there's a downside to them being singles? None of them need scope or need a fresh item
| } | ||
| } | ||
|
|
||
| /* |
| suspend fun getCollectionName(): String | ||
| suspend fun getApiKey(): String | ||
| suspend fun getScheduleId(): String | ||
| suspend fun showVenueMap(): Boolean |
There was a problem hiding this comment.
Why don't we use the showVenueMap declared in Conference?
There was a problem hiding this comment.
Actually it looks like this function isn't removed anywhere, I've removed it.
| @@ -5,14 +5,14 @@ import kotlinx.coroutines.flow.Flow | |||
| import kotlinx.datetime.TimeZone | |||
|
|
|||
| interface ConferenceConfigProvider { | |||
There was a problem hiding this comment.
Could we expose the domain entity instead of fragmenting it into methods that are like a getter?
suspend fun getCurrent(): Conference
fun observeChanges(): Flow<Conference>Looking at the DefaultConferenceConfigProvider, all the methods are basically a "proxy" of the actual currentConference.
By the way, this way it'd look more like a Repository than a Provider, which is kinda vague concept in domain module.
There was a problem hiding this comment.
We could potentially do that, but it would be a somewhat large refactor and it doesn't affect the web support so I'd rather handle that at a later time.
2 participants
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