[codex] reorganize platform folders - #2
Conversation
Tygb99
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment / approval possible
No blocking correctness or security issue found. The folder reorganization and CI path updates look mostly consistent. I found a couple of small documentation/script UX issues worth fixing before or after merge.
⚠️ Warnings
README.md/README.ko.md/docs/MOBILE_APP.md/docs/ko/MOBILE_APP.md— The build examples docd Android, but the following install examples useAndroid/app/build/...from the repository root. If a user runs the install command in the same shell aftercd Android, it resolves asAndroid/Android/app/...and fails.- Suggested fix: use a subshell for the build step:
or explicitly
(cd Android && ./gradlew :app:assembleDebug) "$ANDROID_HOME/platform-tools/adb" install -r Android/app/build/outputs/apk/debug/app-debug.apk
cd ..before the install command.
- Suggested fix: use a subshell for the build step:
💡 Suggestions
Android/scripts/build-windows.ps1:84,Android/scripts/build-windows.ps1:89— Error messages still point to the old setup path:Since the script moved underRun .\scripts\setup-windows-dev.ps1 first.
Android/, this should say:Run .\Android\scripts\setup-windows-dev.ps1 first.
✅ Looks Good
app/→Android/app/and Gradle wrapper/settings migration are clean renames.- GitHub Actions now builds from
Android/and uploadsAndroid/build/artifacts/.... shared/assetsandshared/firmwarepaths are reflected in iOS/shared docs.git diff --check main...HEADpassed during review.- GitHub check-runs for the PR head commit were successful.
- No tracked local planning/build artifacts matched the repo hygiene patterns checked during review.
Validation note
I could not run the local Gradle command in the review container because Java/JDK is not installed there (JAVA_HOME is not set and no java command could be found). I relied on diff review plus the successful GitHub check-runs for build validation.
Reviewed by Hermes Agent.
Tygb99
left a comment
There was a problem hiding this comment.
Re-review Summary
Verdict: LGTM / no blocking issues found
I re-reviewed the updated PR at fefc26a. The previous review notes were addressed:
- Build examples now use
(cd Android && ./gradlew :app:assembleDebug), so the following install commands still resolve from the repository root. Android/scripts/build-windows.ps1now points users to\.\Android\scripts\setup-windows-dev.ps1in the missing Java/SDK error messages.
Validation checked
git diff --check origin/main...HEADpassed.- Tracked-file hygiene pattern check found no matching local/build/secret-style artifacts.
- GitHub PR checks are passing:
Build debug APK— passBuild debug APK— pass
- Searched for stale path references around
scripts/,firmware/bridge-dongle,assets/brand, Gradle build/install paths, andAndroid/Android; no problematic stale references found.
Note
I still could not run local Gradle in this container because Java/JDK is not installed here, but the GitHub Actions build/lint checks for the PR head are green.
Reviewed by Hermes Agent.
Summary
Android/and shared assets/firmware intoshared/.Validation
git diff --check origin/main...HEADgit ls-files | rg -i '(local\.properties|\.apk$|\.aab$|\.jks$|\.keystore$|(^|/)\.env($|\.)|\.DS_Store|KakaoTalk|gpt의견|prd-phonepad|(^|/)build/|(^|/)\.gradle|(^|/)\.kotlin)'returned no tracked matchesAndroid/:./gradlew testDebugUnitTest assembleDebug lintDebug --stacktraceMerge Notes
Android/; the root-level Gradle wrapper moved toAndroid/gradlew.shared/; iOS docs referenceshared/firmware/bridge-dongle/BLETouchMouse/BLETouchMouse.ino.build/,gpt의견/, Android build outputs, PRD drafts, and KakaoTalk CSV exports are intentionally not part of this PR.