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

MVICore multiplatform #114

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

MVICore multiplatform #114

wants to merge 31 commits into from

Conversation

ShikaSD
Copy link
Contributor

@ShikaSD ShikaSD commented Oct 9, 2019

Multiplatform / framework implementation of MVICore.

Consists of several steps:

  • Move current implementation of MVICore to mpp module without any dependencies
    Source / Sink / Cancellable types
    Binder
    BaseFeature / ActorReducerFeature / ReducerFeature
    ⬜️ Middleware
  • Setup build and publishing for mpp modules
  • Create a wrapped version for RxJava based on mpp implementation
  • Deprecate current implementation and replace it with typealiases to help with migration
  • Implement wrapped versions for coroutines and Reaktive
  • Document changes

@ShikaSD ShikaSD requested a review from zsoltk April 14, 2020 14:51
id 'org.jetbrains.kotlin.multiplatform'
}

kotlin {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about iosX64 and iosArm64 targest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can always add more, probably there's also watchos or such?

mvicore-common/build.gradle Outdated Show resolved Hide resolved
@@ -0,0 +1,43 @@
package com.badoo.mvicore.common

interface Sink<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please enable the Kotlin | Style issues | Type parameter can have 'in' or 'out' variance check, set to Weak Wraning.
The T parameter can be in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please check all other places. Having in and out properly specified is very important.

import com.badoo.mvicore.common.Source

interface Connector<Out, In> {
operator fun invoke(source: Source<out Out>): Source<In>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Source should have out specified. Not here.


import com.badoo.mvicore.common.Source

interface Connector<Out, In> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here <out Out, in In>

operator fun invoke(): Source<Action>
}

interface Actor<State, Action, Effect> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how coroutines and Reactive interop will look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can check rx interop, I am pretty sure reaktive will be close

}

companion object {
fun manual() = ManualLifecycle()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have public ManualLifecycle class that can be instantiated directly. Do we want this duplication?

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

Successfully merging this pull request may close these issues.

2 participants