Skip to content

Pass through logging param#21

Merged
horgh merged 3 commits into
mainfrom
greg/fix
Apr 1, 2026
Merged

Pass through logging param#21
horgh merged 3 commits into
mainfrom
greg/fix

Conversation

@oschwald
Copy link
Copy Markdown
Member

@oschwald oschwald commented Apr 1, 2026

  • fix: Forward enableLogging to DeviceDataCollector
  • Add mise setup task for Android SDK provisioning
  • Update some versions

oschwald and others added 3 commits April 1, 2026 12:35
enableLogging from SdkConfig was never passed to DeviceDataCollector,
so all collector-level error logs were silently suppressed even when
logging was explicitly enabled. Add the missing argument and a wiring
test to prevent regression.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`mise run setup` now accepts licenses, installs required platform
packages (android-36, build-tools), and creates local.properties so
the project builds in headless environments without Android Studio.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces environment setup automation using mise, updates the Android SDK target to API 36, and bumps several dependency versions including Ktor and Dokka. It also ensures the enableLogging configuration is correctly passed to the DeviceDataCollector. Feedback was provided regarding the use of reflection in unit tests to access private fields, suggesting the use of internal properties instead to improve test stability.

Comment on lines +279 to +280
val collector = getField<DeviceDataCollector>(tracker, "deviceDataCollector")
val enableLogging = getField<Boolean>(collector, "enableLogging")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using reflection to access private fields in tests is generally discouraged as it makes tests fragile to internal refactoring. Since DeviceDataCollector is an internal class and this test is part of the same module, consider making the enableLogging property internal or providing a package-private getter if you want to avoid reflection while still keeping it hidden from public API consumers.

@horgh horgh merged commit ad9cb69 into main Apr 1, 2026
8 checks passed
@horgh horgh deleted the greg/fix branch April 1, 2026 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants