Add Temporal.toInstant() and simplify androidTimeZoneId() to simplify temporal conversion logic#310
Merged
rfc2822 merged 8 commits intoical4j-3to4from Apr 7, 2026
Merged
Conversation
…cleaner temporal conversions
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors temporal conversion utilities and recurrence-date conversion logic to improve readability and reduce boilerplate when converting between Temporal, Instant, timestamps, and time zones.
Changes:
- Introduces
Temporal.toInstant()and refactors other conversions to use it. - Refactors
androidTimezoneId()to a clearerwhen-style flow and simplifies recurrence date mapping. - Updates tests to use JUnit’s
expectedexceptions instead of manualtry/catch.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lib/src/main/kotlin/at/bitfire/synctools/util/AndroidTimeUtils.kt |
Adds toInstant(), refactors timestamp + timezone conversion logic, and updates exception type. |
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/AndroidRecurrenceMapper.kt |
Simplifies recurrence date conversion via map/when and new conversion helpers. |
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt |
Switches duration alignment helpers to use toInstant() instead of constructing instants from timestamps. |
lib/src/test/kotlin/at/bitfire/synctools/util/AndroidTimeUtilsTest.kt |
Updates exception assertions and aligns tests with the new exception types/conversion approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/AndroidRecurrenceMapper.kt
Outdated
Show resolved
Hide resolved
Temporal.toInstant()
Temporal.toInstant()Temporal.toInstant() and simplify androidTimeZoneId() to simplify temporal conversion logic
cketti
approved these changes
Apr 7, 2026
lib/src/test/kotlin/at/bitfire/synctools/util/AndroidTimeUtilsTest.kt
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request includes several improvements to the temporal conversion logic:
Temporal.toInstant()method and improves exception handlingInstant.ofEpochMilli(toTimestamp())withtoInstant()for cleaner conversionsandroidTimezoneId()to use awhenexpression for better readabilityNo test changes required because the logic is only simplified, but not changed.