Skip to content

Comments

fix: resolve lint and signing issues#1247

Open
WRRicht3r wants to merge 5 commits intodevelopfrom
chore/update-r8
Open

fix: resolve lint and signing issues#1247
WRRicht3r wants to merge 5 commits intodevelopfrom
chore/update-r8

Conversation

@WRRicht3r
Copy link
Contributor

@WRRicht3r WRRicht3r commented Feb 18, 2026

Summary

  • Build tooling: Forced Gradle to pick the published com.android.tools:r8:8.8.34 instead of the missing 8.8.24 artifact (gradle/libs.versions.toml:72, build.gradle:25-36) to keep Kotlin 2.1 metadata compatible. Release notes were expanded with explicit validation and rollout steps so 4.2.0 packaging expectations stay in sync (docs/ releases/4.2.0.md:1-80).

  • Lint/perf fixes: Suppressed the safe vibrator call and removed the incompatible setHasFixedSize usage so lint stops blocking (common/src/main/java/jp/co/soramitsu/common/vibration/DeviceVibrator.kt:1-17, common/src/main/java/jp/co/soramitsu/common/view/bottomSheet/list/dynamic/DynamicListBottomSheet.kt:45-48). Escaped the literal percent signs for APY across the base strings and all translated locales to satisfy StringFormatInvalid (common/src/main/res/values/strings.xml:918-919, along with the corresponding values-pt, values-ru, values-tr, values-vi, values- zh, values-ja, values-in files at their staking string entries).

  • Runtime tests: Updated the ChainAssetRemote fixture to pass coinbaseUrl = null and made the Mockito eq(emptyList()) calls type-safe so Kotlin 2.1 unit tests compile again (runtime/src/test/java/jp/co/soramitsu/runtime/multiNetwork/chain/ChainSyncServiceTest.kt:40-154).

  • Signing workflow: Added keystore resolution helpers that read the release keystore path/password/alias from environment variables or local.properties, then fall back to the repo-local fearless-upload.jks (app/build.gradle:1- 59). Added the new keystore file to .gitignore so it remains untracked (.gitignore:18-20).

Related Issue

Closes # (or) Relates to #

Type of Change

  • feat (new feature)
  • fix (bug fix)
  • refactor (no functional change)
  • chore/build (tooling, CI, deps)
  • docs (README/AGENTS, comments)

Screenshots / Videos

If UI changes, include before/after.

Test Plan

Commands run locally:

./gradlew detektAll
./gradlew runTest
./gradlew :app:lint

Additional checks and scenarios covered:

Risks & Rollout

Potential impact, migrations, or config/secrets required.

Checklist

  • Linked an issue and added a clear description
  • Added/updated tests for changed code (where applicable)
  • Updated docs (README/AGENTS) when behavior or commands changed
  • Ran detektAll, runTest, and :app:lint locally (or via CI)
  • No secrets or local.properties committed

@takemiyamakoto
Copy link

@codex klanker bro, what do you say?

@chatgpt-codex-connector
Copy link

To use Codex here, create an environment for this repo.

@takemiyamakoto
Copy link

@codex review deez

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c7935ad2b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@takemiyamakoto
Copy link

@codex how is this?

@chatgpt-codex-connector
Copy link

I’d call this “close, but not ready to merge yet”.

Quick verdict

The two P2 comments look valid to me, and both can cause real regressions:

  1. APY label regression (%% shown to users).
    The base string currently contains Estimated rewards (%% APY), which will render literally if read as plain text (not String.format-expanded). That means users may indeed see %%. common/src/main/res/values/strings.xmlL918
    The same double-percent pattern appears across localized string files too, so this is not isolated to English. common/src/main/res/values-pt/strings.xmlL796 common/src/main/res/values-ru/strings.xmlL891 common/src/main/res/values-tr/strings.xmlL873 common/src/main/res/values-vi/strings.xmlL903 common/src/main/res/values-zh/strings.xmlL868 common/src/main/res/values-ja/strings.xmlL868 common/src/main/res/values-in/strings.xmlL816

  2. CI signing fallback regression risk.
    ci signing now resolves/falls back to fearless-upload.jks in the repo root, but that file is ignored by git, so fresh clones/partially configured CI can fail to sign release builds if env/local props are missing. app/build.gradleL41-L48 .gitignoreL18-L20


Recommendation

  • Address both P2s before merge.
    • APY strings: switch to single % with formatted="false" (or equivalent safe approach) for plain-text labels.
    • Signing fallback: restore a tracked fallback keystore path (or fail fast with explicit validation) to avoid silent CI breakage.

Commands I used to verify

  • rg --files -g 'AGENTS.md'
  • cat AGENTS.md
  • cat app/AGENTS.md
  • cat common/AGENTS.md
  • nl -ba common/src/main/res/values/strings.xml | sed -n '900,930p'
  • nl -ba app/build.gradle | sed -n '1,90p'
  • nl -ba .gitignore | sed -n '1,40p'
  • rg -n "%% APY|%% TAE|%%" common/src/main/res/values*/*.xml | head -n 40
  • git status --short

View task →

@takemiyamakoto
Copy link

https://github.com/codex how is this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants