Skip to content

refactor: drop two over-engineered seams (enum + stdlib Base64)#5945

Merged
jamesarich merged 1 commit into
mainfrom
claude/intelligent-bhaskara-7c7910
Jun 25, 2026
Merged

refactor: drop two over-engineered seams (enum + stdlib Base64)#5945
jamesarich merged 1 commit into
mainfrom
claude/intelligent-bhaskara-7c7910

Conversation

@jamesarich

Copy link
Copy Markdown
Collaborator

Why

A repo-wide complexity audit surfaced two abstractions that exist only as ceremony. Both are behavior-preserving deletions — fewer lines, same semantics, no new dependency.

🧹 Cleanup

  • ScrollToTopEventenum. It was a sealed class whose only members were two stateless data objects — which is precisely an enum. Converted, and the two if (event is ScrollToTopEvent.X) checks become ==.
  • Deleted Base64Factory. A two-method object that only forwarded to the stdlib kotlin.io.encoding.Base64.Default, with a single call site. Inlined Base64.Default.encode(...) in NodeDetailsSection; the experimental opt-in folds into the @OptIn already on the caller.

Net -15 lines, -1 file.

Left in place (audit candidates rejected after checking)

  • MqttClientSession — backs a real FakeMqttClientSession test seam.
  • AndroidServiceRepository — the @Single annotation-DI anchor; matches every other androidMain class.
  • Desktop Noop* stubs — converting to object is 8 edit sites for -4 lines and no behavior change. Not worth the churn.

Testing Performed

./gradlew spotlessCheck detekt + allTests on every touched module (:core:ui, :feature:node, :feature:messaging, :core:common) — all pass. :core:common 87 tests green.

Two small simplifications surfaced by a repo-wide complexity audit, both
behavior-preserving and verified by spotless/detekt + allTests on every
touched module.

- ScrollToTopEvent: a sealed class whose only members were two stateless
  `data object`s is exactly an enum. Converted; the two `is` checks become
  `==`.
- Base64Factory: a two-method object that only forwarded to the stdlib
  `kotlin.io.encoding.Base64.Default`, with a single call site. Deleted the
  file and inlined `Base64.Default.encode(...)` (opt-in folded into the
  existing @OptIn on the caller).

Net -15 lines, -1 file. Three other audit candidates were left in place after
checking: MqttClientSession backs a real test fake, AndroidServiceRepository
is the @single annotation-DI anchor matching every other androidMain class,
and the desktop Noop* stubs aren't worth the churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🖼️ Preview staleness check — advisory

This PR modifies UI composables but does not update any *Previews.kt files.

Previews power screenshot tests and in-app docs screenshots. Keeping them current ensures visual regression coverage stays accurate.

Changed UI files:

feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt
feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/NodeDetailsSection.kt

What to check:

Pattern Preview file convention
feature/{name}/…/ui/ or component/ feature/{name}/…/*Previews.kt
core/ui/…/ core/ui/…/ (previews colocated)

Adding previews checklist:

  1. Create or update a *Previews.kt file in the same module with @PreviewLightDark
  2. Add @Suppress("PreviewPublic") if the preview is consumed by screenshot-tests
  3. Add corresponding @PreviewTest function in screenshot-tests/src/screenshotTest/
  4. Run ./gradlew :screenshot-tests:updateDebugScreenshotTest to generate reference images

If this PR does not require preview updates (e.g., logic-only change, non-visual refactor), add the skip-preview-check label to dismiss.

@github-actions

Copy link
Copy Markdown
Contributor

📄 Docs staleness check — advisory

This PR modifies user-facing UI source files but does not update any page under docs/en/user/ or docs/en/developer/.

⚠️ Doc changes propagate to 3 consumers: in-app docs browser, Jekyll site (GitHub Pages), and meshtastic.org (Docusaurus sync). Updating a page in docs/en/ automatically flows to all three.

Changed source files:

core/ui/src/commonMain/kotlin/org/meshtastic/core/ui/component/ScrollToTopEvent.kt
feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/ui/contact/Contacts.kt
feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/component/NodeDetailsSection.kt

What to check:

Changed area Likely doc page
feature/messaging/ docs/en/user/messages-and-channels.md
feature/node/ docs/en/user/nodes.md or docs/en/user/node-metrics.md
feature/map/ docs/en/user/map-and-waypoints.md
feature/connections/ docs/en/user/connections.md
feature/settings/ docs/en/user/settings-radio-user.md or docs/en/user/settings-module-admin.md
feature/firmware/ docs/en/user/firmware.md
feature/intro/ docs/en/user/onboarding.md
feature/discovery/ docs/en/user/discovery.md
feature/docs/ Internal docs infrastructure
core/ui/ docs/en/developer/codebase.md or component-specific user pages

New page checklist (if adding a new doc page):

  1. Create the .md file in docs/en/user/ or docs/en/developer/ with last_updated frontmatter
  2. Register in DocBundleLoader.kt with string resources (in-app browser)
  3. Jekyll and Docusaurus sync pick up new pages automatically — no config change needed

If this PR does not require a doc update (e.g., internal refactor, bug fix, test change), add the skip-docs-check label to dismiss this check.

Cross-platform note: This check is advisory while doc coverage matures. Both Android and Apple repos use the same skip-docs-check label and advisory severity. See meshtastic/design standards for shared conventions.

@github-actions github-actions Bot added the refactor no functional changes label Jun 25, 2026
@jamesarich jamesarich added this pull request to the merge queue Jun 25, 2026
Merged via the queue into main with commit ab07347 Jun 25, 2026
22 checks passed
@jamesarich jamesarich deleted the claude/intelligent-bhaskara-7c7910 branch June 25, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor no functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant