Show Dev Settings by default in debug builds

[piotr-iohk]

Description

In order to be consistent with iOS [see] and given the fact that both builds for e2e tests are done in debug configuration the PR sets Dev Settings as enabled in debug mode. E2E tests updated accordingly.

Preview

QA Notes

synonymdev/bitkit-e2e-tests#51

[Copilot]

Pull Request Overview

This PR simplifies the logic for enabling Developer Settings by defaulting it to true in debug builds, aligning with iOS behavior and accommodating E2E test configurations.

Key Changes:

• Replaced complex conditional logic (!Env.isE2eTest && Env.network != Network.BITCOIN) with simpler Env.isDebug check
• Removed unused Network import
• Removed unnecessary @Suppress annotations that are no longer needed

[claude]

Code Review - PR #471: Show Dev Settings by default in debug builds

Overview

This PR simplifies the isDevModeEnabled default value logic in SettingsStore.kt to align with iOS behavior and support E2E testing in debug mode.

---

✅ Positive Findings

1. Code Quality & Simplification

• ✅ Excellent simplification: Removed complex conditional logic !Env.isE2eTest && Env.network != Network.BITCOIN in favor of simple Env.isDebug
• ✅ Cleaner code: Removed unnecessary @Suppress annotations that are no longer needed
• ✅ Removed unused import: org.lightningdevkit.ldknode.Network import properly cleaned up
• ✅ Better alignment: Matches iOS implementation for consistency across platforms

2. Logic & Correctness

• ✅ Correct implementation: BuildConfig.DEBUG is automatically set by the Android build system (true for debug builds, false for release)
• ✅ E2E compatibility: Since E2E tests run in debug configuration, they will correctly have Dev Settings enabled
• ✅ Network independence: Removes coupling between dev mode and network type, which is more intuitive

---

🔍 Observations & Considerations

1. Behavioral Changes
The change modifies when Dev Settings are enabled:

Before:

• Enabled when: NOT in E2E mode AND network is NOT Bitcoin (mainnet)
• This meant: enabled for REGTEST and TESTNET, but disabled for E2E tests and mainnet

After:

• Enabled when: in DEBUG build configuration
• This means: enabled for ALL debug builds (including E2E), disabled for ALL release builds

Impact:

• ⚠️ Release builds: Will now have Dev Settings disabled regardless of network (TESTNET release builds previously had it enabled)
• ✅ Debug builds: All debug builds now consistently have Dev Settings enabled
• ✅ E2E tests: Now have Dev Settings enabled (previously disabled), which aligns with the stated goal

2. Test Coverage

• ℹ️ No unit tests found specifically for SettingsStore or SettingsData default values
• ℹ️ The PR mentions E2E tests were updated accordingly (bitkit-e2e-tests#51)
• 💡 Recommendation: Consider adding a simple unit test to verify the default value behavior:

  @Test
  fun `isDevModeEnabled defaults to true in debug builds`() {
      val settings = SettingsData()
      assertTrue(settings.isDevModeEnabled)
  }

3. User Experience

• ✅ Users can still toggle Dev Settings manually via the cog tap gesture (5 taps) in SettingsScreen.kt:68-88
• ✅ The default only affects fresh installs or reset settings
• ✅ More intuitive: "debug builds = dev features enabled" is a clearer mental model

---

🔒 Security Considerations

• ✅ No security concerns: Dev Settings should be disabled in production release builds, which this change ensures
• ✅ Proper isolation: BuildConfig.DEBUG is set at build time and cannot be manipulated at runtime

---

⚡ Performance Considerations

• ✅ Performance improvement: Simpler boolean check vs. compound conditional
• ✅ Minimal impact: This is a default value assignment with negligible performance implications

---

📝 Suggestions

1. Documentation (Optional)
Consider adding a comment to clarify the behavior:

// Dev mode enabled by default in debug builds for development and E2E testing
val isDevModeEnabled: Boolean = Env.isDebug,

2. Verify Build Variants
Ensure all product flavors (dev, tnet) maintain correct DEBUG flag:

• devDebug ✅ DEBUG=true, NETWORK=REGTEST → Dev Settings enabled
• devRelease ✅ DEBUG=false, NETWORK=REGTEST → Dev Settings disabled
• tnetDebug ✅ DEBUG=true, NETWORK=TESTNET → Dev Settings enabled
• tnetRelease ✅ DEBUG=false, NETWORK=TESTNET → Dev Settings disabled

---

✅ Conclusion

Overall Assessment: Excellent Change

This is a well-considered simplification that:

• ✅ Improves code maintainability
• ✅ Aligns Android with iOS implementation
• ✅ Properly supports E2E testing requirements
• ✅ Maintains security by disabling dev features in release builds
• ✅ Follows Android best practices

Recommendation: Approve ✅

The change is clean, logical, and improves the codebase. The only minor suggestion is to consider adding unit tests for default values to prevent future regressions.

---

🤖 Generated with Claude Code

[ovitrif]

ACK ✅