|
| 1 | +--- |
| 2 | +applyTo: '**' |
| 3 | +--- |
| 4 | + |
| 5 | +# Pull Request Review Guidance |
| 6 | + |
| 7 | +You are reviewing changes for @knighted/develop. Be concise, technical, and specific. Prioritize actionable feedback tied to concrete files and lines. |
| 8 | + |
| 9 | +## Browser support policy |
| 10 | + |
| 11 | +- Target evergreen browsers only (current stable Chrome, Edge, Safari, and Firefox). |
| 12 | +- Do not require old-browser compatibility unless the PR explicitly expands support scope. |
| 13 | +- Do not request legacy polyfills or fallback-only workarounds by default. |
| 14 | + |
| 15 | +## Focus areas |
| 16 | + |
| 17 | +- CDN-first runtime integrity: imports and fallback behavior should remain compatible with src/modules/cdn.js patterns. |
| 18 | +- UI state correctness: drawers, toggles, dialogs, and compact/mobile states should remain synchronized and predictable. |
| 19 | +- BYOT safety: token handling must stay browser-local, avoid leakage, and preserve clear user-facing privacy/removal cues. |
| 20 | +- Accessibility and semantics: form controls, labels, button types, ARIA relationships, and keyboard interactions should be valid. |
| 21 | +- Build and workflow stability: scripts and output behavior should remain consistent unless change is explicitly documented. |
| 22 | +- Tests and docs alignment: behavior changes should update tests and relevant docs. |
| 23 | + |
| 24 | +## What to verify |
| 25 | + |
| 26 | +- No generated artifacts are edited (dist/, coverage/, test-results/). |
| 27 | +- CDN import/fallback behavior is not bypassed with ad hoc URLs in feature modules. |
| 28 | +- Sensitive values (PAT/token) are not logged or exposed in UI/status output. |
| 29 | +- New UI behavior is covered in Playwright where appropriate. |
| 30 | +- Lint/build expectations still pass for changed areas. |
| 31 | + |
| 32 | +## Validation expectations |
| 33 | + |
| 34 | +- Run npm run lint for code and HTML/a11y checks. |
| 35 | +- Run npm run build when touching scripts/, bootstrap/runtime wiring, or import map behavior. |
| 36 | +- For interactive UI changes, confirm behavior in compact/mobile layout and at least one non-default mode. |
| 37 | + |
| 38 | +## Review output format |
| 39 | + |
| 40 | +- Present findings first, ordered by severity. |
| 41 | +- Label each finding as blocking, important, or nit. |
| 42 | +- Include file reference and the minimal fix direction. |
| 43 | +- Keep summary brief and secondary to findings. |
| 44 | + |
| 45 | +## Ask for changes when |
| 46 | + |
| 47 | +- Behavior changes ship without corresponding test updates. |
| 48 | +- New dependencies are added without clear approval. |
| 49 | +- Build/CI/import-map contracts change without docs updates. |
| 50 | +- Accessibility regressions or semantic HTML issues are introduced. |
| 51 | +- Feedback requests are based on unsupported legacy-browser constraints. |
0 commit comments