-
Notifications
You must be signed in to change notification settings - Fork 112
Upgrade the project to Kotlin 1.6.10.
#618
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
Conversation
| const val foundation = "androidx.compose.foundation:foundation:1.1.0-rc01" | ||
| const val material = "androidx.compose.material:material:1.1.0-rc01" | ||
| const val tooling = "androidx.compose.ui:ui-tooling:1.1.0-rc01" | ||
| const val ui = "androidx.compose.ui:ui:1.1.0-rc01" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is the latest version for these libraries, but there's an rc02 for the compiler plugin with Kotlin 1.6.10 support. We'll use the same versions internally very soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you file an issue tracking this, and add a comment here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const val moshi = "com.squareup.moshi:moshi:1.12.0" | ||
| const val adapters = "com.squareup.moshi:moshi-adapters:1.13.0" | ||
| const val codeGen = "com.squareup.moshi:moshi-kotlin-codegen:1.13.0" | ||
| const val moshi = "com.squareup.moshi:moshi:1.13.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support Kotlin 1.6.10.
| public static synthetic fun cancelRuntime$default (Lcom/squareup/workflow1/internal/WorkflowRunner;Ljava/util/concurrent/CancellationException;ILjava/lang/Object;)V | ||
| public final fun nextOutput (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; | ||
| public final fun nextRendering ()Lcom/squareup/workflow1/RenderingAndSnapshot; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler starts spitting out so much more.
|
Is this a draft on purpose? Wondering if I should review yet. |
I personally would wait until we use Kotlin 1.6 internally, otherwise upgrading Workflow internally is blocked. Hence I kept the PR in draft. You could review it now already, though. |
samples/tictactoe/app/src/main/java/com/squareup/sample/mainactivity/TicTacToeComponent.kt
Outdated
Show resolved
Hide resolved
| * We define this otherwise redundant typealias to keep composite workflows | ||
| * that build on [RunGameWorkflow] decoupled from it, for ease of testing. | ||
| */ | ||
| @OptIn(WorkflowUiExperimentalApi::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this is needed, possible b/c of the @WorkflowUiExperimentalApi mistake above? Same question for the other new appearances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler is more strict. It's needed even with the other change.
> Task :samples:tictactoe:common:compileKotlin FAILED
e: /Users/ralf/Development/projects/square/workflow-kotlin/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt: (46, 59): This declaration is experimental and its usage must be marked with '@com.squareup.workflow1.ui.WorkflowUiExperimentalApi' or '@OptIn(com.squareup.workflow1.ui.WorkflowUiExperimentalApi::class)'
…tivity/TicTacToeComponent.kt Co-authored-by: Ray Ryan <[email protected]>
vgonda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me
0xMatthewGroves
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
Red shard is b/c request failures. Green API 21 shard is good enough for me, merging. |
No description provided.