Skip to content

Conversation

@bnvinay92
Copy link
Contributor

@bnvinay92 bnvinay92 commented May 24, 2022

Rundown:

  • Changed the repositoriesMode setting from RepositoriesMode.FAIL_ON_PROJECT_REPOS to RepositoriesMode.PREFER_PROJECT. The multiplatform gradle plugin implicitly adds an ivy() repo to download the kotlin native compiler. Looking for a workaround for this, or at least some way to allow only multiplatform modules on preferring project repos.
  • WorkflowIdentifier was using type: KAnnotatedElement to represent either a KClass or a KType. Explicitly introduced a sealed class WorkflowIdentifierType to emulate this. Equality is now based on the serialized version of this sealed class, which allows save and restore without reflection. KClasses are lost after restoration but since identifiers are only used for comparison in tests this shouldn't break anything.
  • org.jetbrains.kotlinx:kotlinx-coroutines-test:1.5.1 only ships a jvm artifact so bumped only this test dependency in commonTest to 1.6.1 (which ships artifacts for all platforms)

@bnvinay92 bnvinay92 requested review from a team and zach-klippenstein as code owners May 24, 2022 08:29
Copy link
Contributor

@RBusarow RBusarow left a comment

Choose a reason for hiding this comment

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

Thanks for this! It looks great!

I ran this branch's snapshot through the CI on our main project and everything's green. So, as soon as we've added some KDoc to WorkflowIdentifierType, I can approve.

@Suppress("UnstableApiUsage")
dependencyResolutionManagement {
repositoriesMode.set(RepositoriesMode.FAIL_ON_PROJECT_REPOS)
repositoriesMode.set(RepositoriesMode.PREFER_PROJECT)
Copy link
Contributor

Choose a reason for hiding this comment

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

No big deal.

@bnvinay92 bnvinay92 requested a review from RBusarow May 28, 2022 19:32
@bnvinay92
Copy link
Contributor Author

Thanks for this! It looks great!

I ran this branch's snapshot through the CI on our main project and everything's green. So, as soon as we've added some KDoc to WorkflowIdentifierType, I can approve.

Thanks. Added KDoc and removed the redundant dependency here.

Copy link
Contributor

@RBusarow RBusarow left a comment

Choose a reason for hiding this comment

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

Great, thanks!

Copy link
Contributor

@steve-the-edwards steve-the-edwards left a comment

Choose a reason for hiding this comment

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

🥳

@RBusarow RBusarow merged commit f8d97d9 into square:main Jun 1, 2022
@bnvinay92 bnvinay92 deleted the vn/p2-core branch June 2, 2022 14:09
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.

3 participants