Skip to content

Moves Instant properties to :purchases-kmp-core#514

Merged
JayShortway merged 12 commits into
mainfrom
move-instant-properties
Sep 30, 2025
Merged

Moves Instant properties to :purchases-kmp-core#514
JayShortway merged 12 commits into
mainfrom
move-instant-properties

Conversation

@JayShortway

Copy link
Copy Markdown
Member

Description

Round 2 of #469, this time without breaking changes!

  • Deprecates all kotlinx.datetime.Instant properties in :purchases-kmp-datetime, and
  • Adds them as kotlin.time.Instant properties in :purchases-kmp-core (via :purchases-kmp-models).

Also updates the patch version of Kotlin to 2.1.21. We need 2.1.20 for kotlin.time.Instant. 2.1.21 gives us Xcode 16.3 compatibility, up from 16.0.

Closes #468

@JayShortway JayShortway requested a review from a team September 29, 2025 13:51
@JayShortway JayShortway self-assigned this Sep 29, 2025
@JayShortway JayShortway added pr:other pr:force_minor Forces a minor release labels Sep 29, 2025

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! Thanks for handling this 🎉

Comment thread gradle/libs.versions.toml
ios-deploymentTarget-ui = "15.0"
java = "1.8"
kotlin = "2.1.10"
kotlin = "2.1.21"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick. I don't see anything that could cause issues in the changes here, but would it be worth extracting this to a separate PR? Mostly so it shows in the changelog separately and it's clearer when the bump happened.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a great idea! Separate PR is here: #515.

* * For Amazon subscriptions, productsIds are `termSku`.
*/
@ExperimentalTime
public val allExpirationDates: Map<String, Instant?> by lazy {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see we used the instant naming for these in the previous module, like allExpirationInstants, probably to avoid the naming conflict which would cause a breaking change... I kinda liked that the previous name had the type in it, but I don't see a better alternative, and I think this is probably ok, specially considering it's probably the most common way to represent these now 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yea, I wanted to avoid a naming conflict indeed. If they're named the same as the deprecated properties, the new properties shadow the deprecated ones. This means that when developers update purchases-kmp to the version this PR gets released in, they would get build errors because of @ExperimentalTime (because they are unknowingly using the new properties).

@JayShortway JayShortway changed the base branch from main to update-kotlin-2.1.21 September 29, 2025 15:55
Base automatically changed from update-kotlin-2.1.21 to main September 29, 2025 16:13
@JayShortway JayShortway enabled auto-merge (squash) September 29, 2025 16:38
@JayShortway JayShortway changed the title Move Instant properties to :purchases-kmp-core Moves Instant properties to :purchases-kmp-core Sep 29, 2025
@JayShortway JayShortway merged commit ca64f40 into main Sep 30, 2025
11 checks passed
@JayShortway JayShortway deleted the move-instant-properties branch September 30, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:force_minor Forces a minor release pr:other

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update KotlinX Date/Time to 0.7.1

2 participants