[新增] 适用于 Codex 的有关迁移和审查的相关 skill
This commit is contained in:
6 files changed
+425
No files matched your search
@@ -0,0 +1,59 @@
|
||||
---
|
||||
name: legado-compose-review
|
||||
description: Review existing Legado Jetpack Compose code for architecture, behavior, maintainability, and project convention issues. Use when Codex is asked to audit, review, inspect, evaluate, or find problems in Legado Compose screens, routes, ViewModels, contracts, dialogs, sheets, navigation, or early Compose implementations, especially for MVI/UDF, StateFlow/SharedFlow, Clean Architecture, MainActivity navigation, legacy Activity compatibility, and View-era mixed-pattern drift.
|
||||
---
|
||||
|
||||
# Legado Compose Review
|
||||
|
||||
## Overview
|
||||
|
||||
Review existing Compose code before rewriting it. Focus on concrete defects, architectural drift, behavior risks, and missing verification, especially in early Compose screens that may predate the current MVI/UDF, Clean Architecture, and `MainActivity` navigation expectations.
|
||||
|
||||
Read `references/review-checklist.md` for the project-specific checklist and severity guidance.
|
||||
|
||||
## Workflow
|
||||
|
||||
1. Define the review scope.
|
||||
- Identify the exact screen, route, ViewModel, contract, or package under review.
|
||||
- State whether the user wants review only or review plus fixes. If unclear, review first and do not edit code.
|
||||
- Treat unrelated legacy View code as context, not as part of the review, unless it affects the Compose surface.
|
||||
|
||||
2. Build context from code.
|
||||
- Read the `*Screen`, `*ViewModel`, `*Contract`, host Activity/route, DI registration, and any repositories/usecases used by the feature.
|
||||
- Read the old View implementation only if it is still a compatibility caller or behavior reference.
|
||||
- Compare against nearby current examples such as `BookInfo*`, `Search*`, `MainActivity`, and shared components under `ui/widget/components`.
|
||||
|
||||
3. Review by risk, not style preference.
|
||||
- Prioritize behavior regressions, state duplication, lifecycle bugs, navigation bugs, business logic in UI, direct data access from UI/presentation, recomposition hazards, and missing compatibility handling.
|
||||
- Flag style-only issues only when they conflict with established project conventions or make future migration harder.
|
||||
- Do not require broad refactors for a small screen unless the current code creates real behavior or maintenance risk.
|
||||
|
||||
4. Output findings first.
|
||||
- Use code-review style: list findings before summaries.
|
||||
- Include file and tight line references.
|
||||
- Explain the concrete impact and the smallest credible fix.
|
||||
- If using Codex review directives, emit one `::code-comment{...}` per finding.
|
||||
- If no issues are found, say that clearly and mention remaining test/manual verification gaps.
|
||||
|
||||
5. Suggest fixes only after findings.
|
||||
- Group fixes into small, reviewable steps.
|
||||
- For architecture drift, separate compatibility-preserving fixes from larger cleanup.
|
||||
- Recommend `legado-compose-migration` only when the finding implies a migration or rewrite workflow.
|
||||
|
||||
## Review Priorities
|
||||
|
||||
- **P0/P1**: behavior breakage, lost navigation/result behavior, unsafe lifecycle collection, stale state, data corruption, crashes, or compatibility entry points that no longer work.
|
||||
- **P2**: architecture drift that will cause duplicated logic, hard-to-test behavior, recomposition bugs, or incorrect ownership of state/effects.
|
||||
- **P3**: convention drift, maintainability issues, weak naming, missing previews/tests, or small cleanup that should not block behavior.
|
||||
|
||||
## Boundaries
|
||||
|
||||
- Do not rewrite code during a review unless the user asks for fixes.
|
||||
- Do not demand pure architecture where a compatibility boundary is necessary for unreworked View screens.
|
||||
- For new Compose-first code, expect standard Android architecture: `MainActivity` route ownership, ViewModel-owned state, UDF/MVI user actions, `StateFlow`/`SharedFlow`, repositories/usecases, and UI without business logic.
|
||||
- For migrated code, allow a thin retained Activity only for Android `Intent` compatibility with unreworked View callers.
|
||||
- Distinguish MVI `FeatureIntent` user actions from Android `Intent` launch/extras when both appear.
|
||||
|
||||
## Reference
|
||||
|
||||
Read `references/review-checklist.md` for detailed checks. When a review turns into implementation, also read `../legado-compose-migration/references/project-patterns.md` if available.
|
||||
@@ -0,0 +1,4 @@
|
||||
interface:
|
||||
display_name: "Legado Compose Review"
|
||||
short_description: "Review Legado Compose architecture quality"
|
||||
default_prompt: "Use $legado-compose-review to review a Legado Compose screen for MVI, UDF, Clean Architecture, navigation, and project convention issues."
|
||||
@@ -0,0 +1,117 @@
|
||||
# Legado Compose Review Checklist
|
||||
|
||||
## Files to Read
|
||||
|
||||
For a feature review, inspect the smallest complete slice:
|
||||
|
||||
- `FeatureScreen.kt` and any `FeatureSheets.kt` / `FeatureDialogs.kt`.
|
||||
- `FeatureViewModel.kt`.
|
||||
- `FeatureContract.kt` if present.
|
||||
- Host route in `MainActivity.kt` or retained `FeatureActivity.kt`.
|
||||
- Koin registration in `di/appModule.kt`.
|
||||
- Repositories/usecases used by the feature.
|
||||
- Old View caller or XML/adapters only when still used for compatibility or behavior comparison.
|
||||
|
||||
Use current examples as references:
|
||||
|
||||
- `ui/main/MainActivity.kt` for Navigation3 route ownership.
|
||||
- `ui/book/info/BookInfoContract.kt`, `BookInfoViewModel.kt`, `BookInfoScreen.kt` for behavior-heavy MVI/UDF.
|
||||
- `ui/book/search/SearchContract.kt`, `SearchViewModel.kt`, `SearchScreen.kt` for route-level search state.
|
||||
- `ui/widget/components/...` and `ui/theme/...` for shared UI conventions.
|
||||
|
||||
## Architecture Checks
|
||||
|
||||
Flag issues when:
|
||||
|
||||
- A new Compose-first screen is implemented as a standalone Activity instead of a `MainActivity` destination without a compatibility reason.
|
||||
- A retained Activity contains feature business logic instead of acting as a thin Android `Intent` compatibility host.
|
||||
- Business rules, repository calls, DAO calls, persistence writes, or service orchestration live in composables.
|
||||
- ViewModel exposes mutable state directly, exposes `MutableStateFlow`, or lets UI mutate domain objects.
|
||||
- UI state is split across Activity fields, composable `remember` state, adapter state, and ViewModel state in a way that can diverge.
|
||||
- One-shot events such as navigation, result codes, file opening, permission requests, or clipboard writes are represented as persistent `UiState` fields that can replay incorrectly.
|
||||
- MVI `FeatureIntent` user actions are confused with Android `Intent` launch/extras.
|
||||
- Domain/usecase boundaries are bypassed in new screens when a meaningful business action exists.
|
||||
- A migration introduces new domain abstractions for a single trivial UI action without reducing real complexity.
|
||||
|
||||
## UDF and State Checks
|
||||
|
||||
Flag issues when:
|
||||
|
||||
- `Screen` functions own business state instead of receiving `state` and callbacks.
|
||||
- `remember` / `rememberSaveable` stores source-of-truth data that should survive process or route recreation through ViewModel state.
|
||||
- `LaunchedEffect` keys are unstable or cause repeated data loading, duplicate navigation, duplicate toasts, or repeated service calls.
|
||||
- Flows are collected without lifecycle awareness in UI routes where `collectAsStateWithLifecycle()` should be used.
|
||||
- List items lack stable keys where mutation, selection, or animation can make state attach to the wrong row.
|
||||
- Selection, search query, sorting, filtering, loading, and error states are not represented in a single coherent `UiState`.
|
||||
- Derived values are recomputed expensively on every recomposition instead of living in ViewModel state or `remember(key)`.
|
||||
|
||||
## Navigation and Compatibility Checks
|
||||
|
||||
Flag issues when:
|
||||
|
||||
- New Compose destinations skip `MainActivity` Navigation3 route registration.
|
||||
- Legacy Android `Intent` extras or result codes change without an explicit compatibility plan.
|
||||
- A migrated screen is reachable both through `MainActivity` and a retained Activity with inconsistent state initialization.
|
||||
- Navigation is performed directly inside nested composables instead of through callbacks/effects.
|
||||
- Activity Result launchers, file pickers, permission requests, or Android DialogFragments are hidden inside reusable UI composables.
|
||||
- Back behavior bypasses ViewModel decisions when unsaved changes, selection mode, add-to-shelf prompts, or confirmation dialogs exist.
|
||||
|
||||
## UI and Project Convention Checks
|
||||
|
||||
Flag issues when:
|
||||
|
||||
- Raw Material components ignore `LegadoTheme` or existing shared components where the project already has a wrapper.
|
||||
- A screen recreates shared UI already present under `ui/widget/components`.
|
||||
- User-facing text is hardcoded instead of using string resources, except for temporary debug-only text.
|
||||
- Compose UI keeps ViewBinding, adapter, or XML assumptions after migration.
|
||||
- Dialogs and bottom sheets use inconsistent project components when `AppAlertDialog`, `AppModalBottomSheet`, or existing option sheets fit.
|
||||
- Image loading bypasses existing Coil cover/image helpers where cover behavior, cache, SVG, or GIF handling matters.
|
||||
|
||||
## Clean Architecture Checks
|
||||
|
||||
For new screens, expect:
|
||||
|
||||
- Compose renders state and emits user actions.
|
||||
- ViewModel owns state, intent handling, and effect emission.
|
||||
- Usecases own reusable business actions.
|
||||
- Repositories mediate data access.
|
||||
- DAOs remain behind repositories unless adding a boundary would be disproportionate and local project patterns already allow direct use.
|
||||
|
||||
For migrated screens, allow pragmatic intermediate code only when:
|
||||
|
||||
- It preserves behavior.
|
||||
- It is isolated behind a clear compatibility boundary.
|
||||
- It does not spread View-era assumptions into new Compose-first code.
|
||||
|
||||
## Output Template
|
||||
|
||||
Use this shape unless the user asks for another format:
|
||||
|
||||
```text
|
||||
Findings
|
||||
- [P1] Title
|
||||
file:line
|
||||
Impact: ...
|
||||
Fix: ...
|
||||
|
||||
Open Questions
|
||||
- ...
|
||||
|
||||
Notes
|
||||
- No issues found in ... / Tests not run ...
|
||||
```
|
||||
|
||||
When using Codex inline review comments, emit one directive per finding with tight line ranges:
|
||||
|
||||
```text
|
||||
::code-comment{title="[P2] Keep route state in ViewModel" body="..." file="/absolute/path/FeatureScreen.kt" start=42 end=45 priority=2 confidence=0.8}
|
||||
```
|
||||
|
||||
## Verification Suggestions
|
||||
|
||||
Recommend verification based on the reviewed change:
|
||||
|
||||
- Kotlin-only review/fix: `.\gradlew.bat :app:compileAppDebugKotlin`.
|
||||
- Resource/XML/manifest impact: `.\gradlew.bat :app:assembleAppDebug`.
|
||||
- Navigation or compatibility changes: manually open both `MainActivity` route and any retained legacy Android `Intent` entry point.
|
||||
- Behavior-heavy ViewModel changes: add or run focused tests only where the project has a practical test seam.
|
||||
Reference in new issue
Block a user