PR Review: #$0
If $0 is empty or not a number, ask the user for a PR number before proceeding.
Context
Fetch the PR details:
- Diff: !
gh pr diff $0 - PR info: !
gh pr view $0 - Changed files: !
gh pr diff $0 --name-only
Review Checklist
For each changed file, check the applicable sections below. Report findings grouped by severity: Blocking (must fix), Suggestion (should fix), Nit (optional).
1. Angular 22+ Patterns
- New code uses
input()/output()/model()/linkedSignal(). Existing@Input()/@Output()decorators being modified should prefer migration to signal functions, but it's not blocking. -
host: {}in decorator — no@HostBinding()/@HostListener() -
@if/@for/@switch— no*ngIf/*ngFor/*ngSwitch - No
standalone: truein@Component(default since Angular 19) - No
allowSignalWritesoption ineffect()(the option no longer exists) -
DestroyRef+takeUntilDestroyed()— no customDestroyedService -
computed()+host: { '[class]': }— noCssClassBuilder/@applyCssClass - Member ordering: decorated props → signal inputs/outputs → public → protected → private → constructor → methods
2. State Management
-
signal()only used when a reactive consumer exists (template, computed, effect, host binding) - Plain properties for internal bookkeeping, one-time flags, cached values
- No redundant
markForCheck()after signal updates -
BehaviorSubject→signal()where there are no async consumers -
effect()for signal reactions; RxJS only for async operations (HTTP, WebSocket, timers) - No
effect()used for state derivation — usecomputed()orlinkedSignalinstead -
linkedSignalused where mutable derived state is needed (noteffect()+signal.set()) - No object/array mutation in place then
signal.set()with same reference — always new references - No conditional signal reads creating invisible dependency gaps in
effect()/computed()
3. Dependency Injection
-
InjectionTokenfor contextual defaults (not@ContentChildassigning to signal inputs) - Token defined near child component, injected with
{ optional: true } -
FD_prefix for component identity tokens -
contentChild()/contentChildren()query by token, not concrete class
4. Unit Tests
- New/changed inputs, outputs, or behavior have corresponding test updates
- Tests cover user interactions and realistic scenarios, not implementation details
- No tests for TypeScript-prevented scenarios (e.g. passing undefined to required input)
- No tests for Angular DI-prevented scenarios (e.g. missing required dependency)
-
fixture.componentRef.setInput()used for signal inputs in tests - Individual component imports in test
importsarrays — no deprecated*Moduleclasses - Unique test component names (no generic
TestComponent)
5. Documentation Examples
- If public API changed (new input, changed behavior, removed feature), docs examples are updated
- Examples consolidated into existing examples — no unnecessary standalone example files
- Every demonstrated feature is user-observable (can the user see it working?)
-
@fundamental-styles/common-cssutility classes (sap-flex,sap-margin-*,sap-padding-*) — no inline styles - Individual component imports in examples — no deprecated
*Moduleclasses
6. Breaking Changes
- If exports removed, inputs/outputs renamed, or defaults changed:
- Commit message has
!after scope:fix(core)!: description - Commit message has
BREAKING CHANGE:footer with migration instructions - PR description has a "Breaking Changes" section with before/after examples
- Commit message has
- Widely-used APIs marked
@deprecatedbefore removal (not removed in same PR) - All usages searched with grep before removing exports
7. Commit & PR Format
- Commit format:
<type>(<scope>): <subject> - Valid type:
feat|fix|docs|style|refactor|test|build|ci|chore - Valid scope:
core|platform|cdk|btp|cx|i18n|datetime-adapter|ui5|docs|e2e|ci - PR title follows same format
- No
WIPprefix (unless intentionally draft)
Output Format
Summarize findings as:
## Review Summary
**PR:** #$0
**Overall:** APPROVE / REQUEST CHANGES / COMMENT
### Blocking
- [file:line] Issue description
### Suggestions
- [file:line] Issue description
### Nits
- [file:line] Issue description
### Missing
- Tests: list any untested new behavior
- Docs: list any undocumented API changes
- Breaking changes: list any unannounced breaking changes