chore: address PR feedback on domain migration

- Add distinctUntilChanged() to prevent duplicate emissions when
  DataStore write triggers re-collection (fixes Copilot review comment)
- Improve runBlocking comment to explain why it's acceptable here:
  one-time migration, fast DataStore write, downstream deduplication
- Rename test from 'no repeated saves' to 'idempotent across multiple reads'
  to accurately reflect what the test verifies (fixes Copilot review comment)

Ref: https://github.com/usetrmnl/trmnl-android/pull/262
All 49 unit tests pass (0 failures, 0 errors)
This commit is contained in:
Hossain Khan
2026-02-08 20:21:19 -05:00
parent b355021b6f
commit 545f8fd810
2 changed files with 7 additions and 4 deletions
@@ -294,8 +294,11 @@ class TrmnlDeviceConfigDataStore
"Migrating API base URL from ${config.apiBaseUrl} to $newUrl for TRMNL device",
)
val migratedConfig = config.copy(apiBaseUrl = newUrl)
// Save the migrated config back to DataStore synchronously
// Using runBlocking is acceptable here as this is a one-time migration
// Persist the migrated config back to DataStore (one-time migration)
// Note: Using runBlocking here as Flow.map doesn't support suspend operations.
// This is acceptable because: (1) it's a one-time migration per user,
// (2) DataStore writes are fast, and (3) distinctUntilChanged() below
// prevents duplicate downstream emissions from the save triggering re-collection.
runBlocking {
saveDeviceConfig(migratedConfig)
}
@@ -303,7 +306,7 @@ class TrmnlDeviceConfigDataStore
} else {
config
}
}
}.distinctUntilChanged()
/**
* Saves the complete device configuration
@@ -821,7 +821,7 @@ class TrmnlDeviceConfigDataStoreTest {
}
@Test
fun `deviceConfigFlow migration is idempotent - no repeated saves`() =
fun `deviceConfigFlow migration is idempotent across multiple reads`() =
runTest {
// Arrange
val config =