Skip to content

Commit 65f9f47

Browse files
frettclaude
andcommitted
Add three checklist items to pr-review skill from PR comment analysis
- eventSink must be passed as a direct lambda, not a local variable - navigator.pop() must be inside the launch block after async operations - Use Fakes instead of mockk for interfaces with @composable functions Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 35cef65 commit 65f9f47

1 file changed

Lines changed: 10 additions & 0 deletions

File tree

.claude/skills/pr-review/SKILL.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,15 @@ To dismiss a finding so it won't appear in future reviews, say:
119119
- [ ] `UiEvent` is a `sealed interface` implementing `CircuitUiEvent`, marked `internal`
120120
- [ ] `UiState` and `UiEvent` defined as nested types inside the Presenter
121121
- [ ] `UiState` exposes `val eventSink: (UiEvent) -> Unit`
122+
- [ ] `eventSink` lambda passed directly when constructing `UiState` — not extracted to a local variable first:
123+
```kotlin
124+
// Correct
125+
return UiState(items = items) { event -> when (event) { … } }
126+
// Wrong
127+
val eventSink: (UiEvent) -> Unit = { … }
128+
return UiState(items = items, eventSink = eventSink)
129+
```
130+
- [ ] `navigator.pop()` (or any navigation call) placed *inside* the `launch { }` block when it follows an async operation — `rememberCoroutineScope()` is canceled on composition disposal and can cancel an in-flight write before it completes
122131
- [ ] Presenter contains no UI logic — pure state/event handling
123132

124133
**UI Composable**
@@ -157,6 +166,7 @@ To dismiss a finding so it won't appear in future reviews, say:
157166
### Testing
158167

159168
- [ ] Presenter tests use `presenterTestOf { }` (Circuit test API)
169+
- [ ] Interfaces with `@Composable` functions use a hand-written `Fake*` class in tests, not `mockk<>()` — mockposable delays Kotlin version uptake and is incompatible with newer compiler versions
160170
- [ ] Paparazzi tests extend `BasePaparazziTest` from `:ui:base` testFixtures with `@TestParameter` night/accessibility matrix
161171
- [ ] Snapshots not recorded locally — triggered via GitHub Actions workflow on the feature branch
162172

0 commit comments

Comments
 (0)