Drop AndroidCompatTimeZoneRegistry#233
Merged
rfc2822 merged 9 commits intoical4j-3to4from Mar 17, 2026
Merged
Conversation
- Rename `usedTimeZones` to `usedTimezoneIds` and change type from `Set<ZoneId>` to `Set<String>` - Update `timeZonesOf()` to extract TZID parameters instead of ZoneId from ZonedDateTime - Add recursive processing of subcomponents in `timeZonesOf()` - Add TODO comments for future improvements - Remove unused `toZonedDateTime()` helper function - Update imports and add `@VisibleForTesting` annotation - Add TODO comment in test file for missing tests
- Delete AndroidCompatTimeZoneRegistry implementation and factory - Remove AndroidCompatTimeZoneRegistryTest and Ical4jConfigurationTest - Update ical4j.properties to remove custom timezone registry - Remove ProGuard rules for AndroidCompatTimeZoneRegistry
- Add test case for Europe/Kiev timezone handling - Ensure VTIMEZONE TZID matches original Android timezone ID - Add logging for timezone ID mismatches between Android and ical4j - Update imports and add logger to ICalendarGenerator
- Add `copyVTimeZone` method to safely modify VTIMEZONE without affecting registry cache - Replace direct timezone modification with copy-based approach - Add test for `copyVTimeZone` to verify property list and observances isolation - Update timezone ID replacement test to use regex pattern matching - Add imports for new functionality
- Implement tests for extracting TZIDs from date properties - Add test for empty result when no TZIDs present - Include test for TZID extraction from subcomponents - Add test for empty component case - Remove unused helper function `toZonedDateTime`
- Introduce new `VTimeZoneMinifier` class with dedicated minification logic - Move `minifyVTimeZone` method from `ICalendar` to new class - Add comprehensive tests for `VTimeZoneMinifier` functionality - Remove now redundant test cases from `ICalendarTest` - Clean up imports and unused helper functions
- Rename `minifyVTimeZone` to `minify` and change parameter type from `ZonedDateTime?` to `Temporal?` - Add `asZonedDateTime` helper function to convert various Temporal types to ZonedDateTime - Update all test cases to use new method name and handle nullable start time - Fix typo in test comment - Integrate minifier into ICalendarGenerator
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the AndroidCompatTimeZoneRegistry integration and moves timezone minification logic into a dedicated VTimeZoneMinifier, wiring it into ICalendarGenerator while adding/adjusting tests to validate timezone handling (including TZID alias preservation like Europe/Kiev).
Changes:
- Removed
AndroidCompatTimeZoneRegistry(and configuration/tests/proguard rules referencing it). - Added
VTimeZoneMinifierand migrated minification behavior out ofICalendar. - Updated
ICalendarGeneratorto collect TZIDs from properties, preserve original Android TZIDs when ical4j resolves an alias, and added tests forcopyVTimeZoneandtimeZonesOf.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/test/kotlin/at/bitfire/synctools/icalendar/VTimeZoneMinifierTest.kt | New unit tests covering VTIMEZONE minification behavior across several zones. |
| lib/src/test/kotlin/at/bitfire/synctools/icalendar/ICalendarGeneratorTest.kt | Adds tests for TZID alias preservation, VTIMEZONE copying safety, and timeZonesOf. |
| lib/src/test/kotlin/at/bitfire/ical4android/Ical4jConfigurationTest.kt | Removes test that asserted the custom registry factory was configured. |
| lib/src/test/kotlin/at/bitfire/ical4android/ICalendarTest.kt | Removes timezone-minification tests that previously exercised ICalendar.minifyVTimeZone. |
| lib/src/main/resources/ical4j.properties | Stops configuring ical4j to use AndroidCompatTimeZoneRegistry. |
| lib/src/main/kotlin/at/bitfire/synctools/icalendar/VTimeZoneMinifier.kt | Introduces standalone VTIMEZONE minification supporting multiple Temporal types. |
| lib/src/main/kotlin/at/bitfire/synctools/icalendar/ICalendarGenerator.kt | Integrates minifier, adds TZID alias preservation via copyVTimeZone, and implements timeZonesOf. |
| lib/src/main/kotlin/at/bitfire/ical4android/ICalendar.kt | Removes minifyVTimeZone from ICalendar.Companion. |
| lib/src/main/kotlin/at/bitfire/ical4android/AndroidCompatTimeZoneRegistry.kt | Deletes the registry wrapper implementation. |
| lib/src/androidTest/kotlin/at/bitfire/ical4android/AndroidCompatTimeZoneRegistryTest.kt | Deletes instrumentation tests for the removed registry. |
| lib/consumer-rules.pro | Removes keep rules for the removed registry classes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
lib/src/main/kotlin/at/bitfire/synctools/icalendar/VTimeZoneMinifier.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/at/bitfire/synctools/icalendar/VTimeZoneMinifierTest.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/at/bitfire/synctools/icalendar/VTimeZoneMinifierTest.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/icalendar/ICalendarGenerator.kt
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/icalendar/ICalendarGenerator.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/at/bitfire/synctools/icalendar/ICalendarGeneratorTest.kt
Outdated
Show resolved
Hide resolved
- Enhance `minify` method to handle timezone conversion more robustly - Add fallback to system default timezone when original TZID is invalid - Remove redundant null checks and simplify logic flow - Fix test function name formatting - Update documentation to reflect parameter name changes - Clean up code comments and remove outdated TODOs - Add null safety for timezone registry lookups - Improve error handling in timezone minification validation -
…attimezoneregistry
rfc2822
deleted the
198-ical4j-4x-update-drop-androidcompattimezoneregistry
branch
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.
AndroidCompatTimeZoneRegistryand related test filesICalenderGeneratornow preserves original Android timezone IDs when adding VTIMEZONEs (this was previously the reason whyAndroidCompatTimeZoneRegistryexisted)PR looks bigger than it is because it moves VTIMEZONE minification around (with little changes).