refactor: unify duplicate selectAppInteractively implementations - #10844
Conversation
### Description Consolidates the three duplicate implementations of `selectAppInteractively` found in `src/commands/apps-sdkconfig.ts`, `src/init/features/ailogic/index.ts`, and `src/management/apps.ts` into a single, public, type-safe implementation exported from `src/management/apps.ts`. Key Changes: - Replaced the private implementations in `src/commands/apps-sdkconfig.ts` and `src/init/features/ailogic/index.ts` with the exported function from `src/management/apps.ts`. - Removed unused imports and duplicate helper functions like `checkForApps`. - Added unit tests for `selectAppInteractively` covering empty checks, choice formatting, and custom message options. - Adjusted unit tests in `src/init/features/ailogic/index.spec.ts` to expect the new general project app check error message. ### Scenarios Tested - Ran mocha tests on app management (`src/management/apps.spec.ts`) and AI logic (`src/init/features/ailogic/index.spec.ts`). - Ran complete CLI fast test suite with 4685 unit tests. - Linted modified files to ensure strict TypeScript checks and formatting guidelines. ### Sample Commands `npx mocha src/management/apps.spec.ts` `npx mocha src/init/features/ailogic/index.spec.ts`
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the interactive app selection logic by moving selectAppInteractively and checkForApps to a shared utility in src/management/apps.ts, reducing duplication across commands and initialization features. It also adds corresponding unit tests. The feedback highlights several empty JSDoc blocks added to exported functions in src/management/apps.ts that should either be removed or populated with meaningful descriptions.
| /** | ||
| * | ||
| */ |
There was a problem hiding this comment.
Several empty JSDoc blocks have been added to exported functions in this file. These empty comments do not provide any useful documentation and add unnecessary noise to the codebase.
Please either remove them or populate them with meaningful descriptions of the functions.
Other occurrences in this file:
- Lines 170-172 (
sdkInit) - Lines 190-192 (
getSdkOutputPath) - Lines 210-212 (
checkForApps) - Lines 262-264 (
getSdkConfig) - Lines 631-633 (
writeConfigToFile) - Lines 778-780 (
findIntelligentPathForIOS) - Lines 811-813 (
findIntelligentPathForAndroid)
| /** | |
| * | |
| */ | |
| /** | |
| * Detects the app platform based on the directory contents, prompting the user if needed. | |
| */ |
| displayName: string; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Remove these empty comment blocks. Applies throughout the file
3 participants
Description
Consolidates the three duplicate implementations of
selectAppInteractivelyfound insrc/commands/apps-sdkconfig.ts,src/init/features/ailogic/index.ts, andsrc/management/apps.tsinto a single, public, type-safe implementation exported fromsrc/management/apps.ts.Key Changes:
src/commands/apps-sdkconfig.tsandsrc/init/features/ailogic/index.tswith the exported function fromsrc/management/apps.ts.checkForApps.selectAppInteractivelycovering empty checks, choice formatting, and custom message options.src/init/features/ailogic/index.spec.tsto expect the new general project app check error message.Scenarios Tested
src/management/apps.spec.ts) and AI logic (src/init/features/ailogic/index.spec.ts).Sample Commands
npx mocha src/management/apps.spec.tsnpx mocha src/init/features/ailogic/index.spec.ts