Skip to content

Conversation

sunkup
Copy link
Member

@sunkup sunkup commented Jul 22, 2025

Purpose

Refactor to make service detection and collection list refresh easier to understand.

Short description

No code has been changed besides moving it around.

  • Extract discoverHomesets() method from CollectionListRefresher to new ServiceRefresher class
  • Adapt usage in RefreshCollectionsWorker
  • Extract tests to separate class too.

Note that we duplicate TestDispatcher on purpose (instead of extracting it to separate file and reusing it) in order to further isolate the tests and make the smaller TestDispatcher occurrences easier to understand.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@sunkup sunkup requested review from ArnyminerZ and Copilot July 22, 2025 09:45
@sunkup sunkup self-assigned this Jul 22, 2025
@sunkup sunkup added the refactoring Internal improvement of existing functions label Jul 22, 2025
@sunkup sunkup linked an issue Jul 22, 2025 that may be closed by this pull request
Copy link
Contributor

@Copilot 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 extracts service detection functionality from CollectionListRefresher into a new dedicated ServiceRefresher class to improve code organization and maintainability. The main changes separate concerns by moving home set discovery logic into its own specialized class.

  • Extract discoverHomesets() method and related properties from CollectionListRefresher to new ServiceRefresher class
  • Update RefreshCollectionsWorker to use the new ServiceRefresher via dependency injection
  • Move corresponding tests from CollectionListRefresherTest to new ServiceRefresherTest class

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ServiceRefresher.kt New class containing extracted home set discovery logic with assisted injection factory
RefreshCollectionsWorker.kt Updated to inject and use ServiceRefresher.Factory instead of calling CollectionListRefresher directly
CollectionListRefresher.kt Removed discoverHomesets() method and related properties/imports that were moved to ServiceRefresher
ServiceRefresherTest.kt New test class with extracted home set discovery tests
CollectionListRefresherTest.kt Removed testDiscoverHomesets() test method that was moved to ServiceRefresherTest

@sunkup sunkup marked this pull request as ready for review July 22, 2025 09:46
Copy link
Member

@rfc2822 rfc2822 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, only small comments

@sunkup sunkup requested a review from rfc2822 July 24, 2025 09:26
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Nice :)

@rfc2822 rfc2822 merged commit 288583b into main-ose Jul 24, 2025
7 checks passed
@rfc2822 rfc2822 deleted the 1589-create-servicerefresher-that-contains-discoverhomesets branch July 24, 2025 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Internal improvement of existing functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create ServiceRefresher that contains discoverHomesets().
2 participants