Skip to content

Event: extract EventReader/EventWriter#41

Merged
rfc2822 merged 4 commits intomainfrom
eventwriter
Jul 10, 2025
Merged

Event: extract EventReader/EventWriter#41
rfc2822 merged 4 commits intomainfrom
eventwriter

Conversation

@rfc2822
Copy link
Member

@rfc2822 rfc2822 commented Jul 9, 2025

Event is a data class and as such it shouldn't have hidden logic.

This PR extracts reading/writing Events into respective classes.

(In a later stage of refactoring, we will be able to completely remove the Event class and map directly between iCalendar ↔ Android data rows.)

To be merged for bitfireAT/davx5-ose#1573

@rfc2822 rfc2822 added the refactoring Quality improvement of existing functions label Jul 9, 2025
@rfc2822 rfc2822 requested a review from Copilot July 9, 2025 16:06

This comment was marked as outdated.

@rfc2822 rfc2822 changed the title Extract EventWriter Event: extract EventReader/EventWriter Jul 9, 2025
@rfc2822 rfc2822 self-assigned this Jul 9, 2025
@rfc2822 rfc2822 requested a review from Copilot July 9, 2025 19:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR pulls the reading and writing logic out of the Event data class into new EventReader and EventWriter classes, updating all tests to use the new APIs and removing hidden behavior from Event.

  • Introduces EventReader/EventWriter and migrates all Event.eventsFromReader/Event.write calls.
  • Removes read/write methods and companion object from Event.
  • Updates unit and instrumentation tests to use the new reader/writer and cleans up legacy code.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
EventValidatorTest.kt Replaced static reader calls with eventReader.readEvents; added eventReader property.
EventWriterTest.kt New tests validating EventWriter.write output.
EventTest.kt (unit tests) Added tests for Event.toString and organizerEmail accessor.
EventReaderTest.kt Renamed test class, removed legacy code, switched to EventReader.readEvents.
EventWriter.kt New class extracting write logic from Event.
EventReader.kt New class extracting read logic from Event.
Event.kt Stripped out read/write logic and companion object from Event.
Ical4jTest.kt (androidTest) Updated to call EventReader.readEvents.

@rfc2822 rfc2822 marked this pull request as ready for review July 9, 2025 19:48
@rfc2822 rfc2822 requested a review from sunkup July 9, 2025 19:48
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@rfc2822 rfc2822 merged commit 503324b into main Jul 10, 2025
9 checks passed
@rfc2822 rfc2822 deleted the eventwriter branch July 10, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Quality improvement of existing functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants