11---
2- description: Apply when reviewing code changes, running git diff, or preparing a PR
2+ description: Apply when preparing a PR, reviewing code changes, running git diff, or generating a changeset
33alwaysApply: false
44---
55
6- # Code Review Rules
6+ # PR Workflow & Code Review
77
8- It is a local code review process done before submitting a PR.
8+ ## Git & PR Workflow
99
10- ## Process
10+ | Requirement | Details |
11+ | --- | --- |
12+ | **No force-push after review** | GitHub loses review history; use merge commits instead |
13+ | **Changelog required** | Run `pnpm changelog` before merge |
14+ | **Conventional commits** | PR title must follow conventional commit format |
15+ | **Separate concerns** | Unrelated changes go in separate PRs; easier revert and isolation |
16+ | **Changeset messages are user-facing** | Write "Add X to Y" or "Fix X in Y", not implementation details |
17+ | **`skip changelog` label** | Use only for non-user-facing changes (CI, tests, docs); never for bug fixes |
18+ | **Use `--frozen-lockfile`** | Run `pnpm install --frozen-lockfile` to avoid unnecessary lockfile changes |
19+ | **Deprecation rules** | Add to `@drivenets/eslint-plugin-design-system` when deprecating |
20+
21+ ```markdown
22+ <!-- Good changeset message -->
23+ Add DsCard component
24+
25+ <!-- Bad changeset message - implementation details -->
26+ Refactor card to use CSS modules and fix hover state selector specificity
27+ ```
28+
29+ ---
30+
31+ ## Code Review Process
1132
12331. Get diff: `git diff origin/main`
13- 2. Review every changed file against project cursor rules
34+ 2. Review every changed file against project cursor rules (scss, react-patterns, storybook, etc.)
14353. Flag only clear, high-severity issues (max 10 inline comments)
1536
16- ## Inline comment format
37+ ### Inline comment format
1738
1839```typescript
1940/**
@@ -23,19 +44,24 @@ It is a local code review process done before submitting a PR.
2344
2445- One issue per comment; place on the exact changed line
2546- Natural tone, specific and actionable; do not mention automated or high-confidence
26- - Severity emojis: 🚨 Critical 🔒 Security ⚡ Performance ⚠️ Logic ✨ Improvement
27-
28- ## Priorities to check
29-
30- - No `!important` in SCSS
31- - No hardcoded colors or spacing (use `--color-*`, `--spacing-*` tokens)
32- - No `:global` in CSS modules
33- - No unnecessary `useMemo`/`useCallback`
34- - No `forwardRef` (deprecated)
35- - No cross-component internal imports
36- - No `useLayoutEffect` without DOM reads
37- - Stories have `play` functions with `fn()` and a11y queries
38- - No inline styles in stories
39- - Props layer doesn't expose library internals
40- - Logic errors, security issues, race conditions, missing edge cases
47+ - Severity: 🚨 Critical 🔒 Security ⚡ Performance ⚠️ Logic ✨ Improvement
48+
49+ ---
50+
51+ ## PR Checklist
52+
53+ Before submitting a PR:
4154
55+ - [ ] Changelog added (`pnpm changelog`) with user-facing message
56+ - [ ] Behavioral coverage in `*.browser.test.tsx` where interactions matter
57+ - [ ] Browser tests use a11y queries (no test-ids unless unavoidable)
58+ - [ ] CSS uses design tokens, no `!important`
59+ - [ ] No inline styles in stories
60+ - [ ] Props layer doesn't expose library internals
61+ - [ ] Types exported from `.types.ts` with variant arrays
62+ - [ ] Code is well-spaced and readable
63+ - [ ] Matches Figma design
64+ - [ ] Storybook examples show all states (controlled + localized)
65+ - [ ] No cross-component internal imports
66+ - [ ] No unnecessary `useMemo`/`useCallback`
67+ - [ ] Check Ark UI for existing primitives before custom implementation
0 commit comments