Conversation
c487ad9 to
87a6c57
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the event-building logic for calendar synchronization by extracting field mapping logic from the monolithic LegacyAndroidEventBuilder2 class into dedicated builder classes. This improves maintainability and extensibility while preserving existing functionality.
Key Changes:
- Extracted field-specific mapping logic into dedicated builder classes (
AttendeesBuilder,CategoriesBuilder,DescriptionBuilder, etc.) - Refactored
LegacyAndroidEventBuilder2to use these modular builders - Added comprehensive unit tests using Robolectric for the new builder classes
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AndroidEntityBuilder.kt |
Renamed interface from AndroidEventFieldBuilder to AndroidEntityBuilder and updated documentation |
TitleBuilder.kt |
Updated to implement the renamed AndroidEntityBuilder interface |
AttendeesBuilder.kt |
New builder class for mapping event attendees to Android Entity |
CategoriesBuilder.kt |
New builder class for mapping event categories to extended properties |
DescriptionBuilder.kt |
New builder class for mapping event description to Android Events field |
LocationBuilder.kt |
New builder class for mapping event location to Android Events field |
RemindersBuilder.kt |
New builder class for mapping event alarms/reminders |
RetainedClassificationBuilder.kt |
New builder class for preserving non-standard classification values |
UnknownPropertiesBuilder.kt |
New builder class for mapping unknown iCalendar properties |
UrlBuilder.kt |
New builder class for mapping event URL to extended properties |
LegacyAndroidEventBuilder2.kt |
Refactored to use new builder classes, removing duplicated mapping logic |
| Various test files | Added comprehensive unit tests for each new builder class |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...st/kotlin/at/bitfire/synctools/mapping/calendar/builder/RetainedClassificationBuilderTest.kt
Show resolved
Hide resolved
Merged
sunkup
approved these changes
Sep 22, 2025
Member
sunkup
left a comment
There was a problem hiding this comment.
There could be more kdoc on the builder classes for my taste, but we can add than when we look at them in detail :)
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 refactors the event-building logic for calendar synchronization by modularizing the mapping of event fields into dedicated builder classes. The main construction logic in
LegacyAndroidEventBuilder2is simplified and extended to use these new builder classes, which improves maintainability and extensibility.Mapping logic of the fields or tests is not changed.
AttendeesBuilder,CategoriesBuilder,DescriptionBuilder,LocationBuilder,RemindersBuilder,RetainedClassificationBuilder,UnknownPropertiesBuilder,UrlBuilder) that each handle mapping a specific aspect of an event to an AndroidEntity.When this is merged, #71 is the next one.