-
Notifications
You must be signed in to change notification settings - Fork 3
Expand file tree
/
Copy pathcode-review.mdc
More file actions
41 lines (25 loc) · 1.92 KB
/
code-review.mdc
File metadata and controls
41 lines (25 loc) · 1.92 KB
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
---
description: PR review checklist for @contentstack/utils — API docs, compatibility, security, testing
alwaysApply: true
---
# Code review checklist — `@contentstack/utils`
Use severity labels (**Blocker / Major / Minor**) when triaging findings.
## Public API and documentation
- **Blocker/Major:** New or changed **exports** from `src/index.ts` need accurate **JSDoc** (or clear type names) matching runtime behavior.
- **Major:** README / CHANGELOG updates when behavior is user-visible or migration is needed.
## Backward compatibility
- **Blocker:** Unplanned breaking changes to **function signatures** or **default behavior** consumed by Delivery SDK integrations or documented `renderOption` contracts.
- **Major:** Stricter throwing on inputs that previously passed (especially `getContentstackEndpoint`, RTE traversals).
## Errors
- This package uses **plain `Error`** (e.g. `endpoints.ts`); new code should keep messages actionable. **Major:** Silent failures where callers need to detect bad input.
## Null safety and RTE edge cases
- **Major:** Missing guards on **null/undefined** node or entry fragments (historically sensitive in `entry-editable` / RTE paths).
- **Minor:** Align with **`strictNullChecks: false`** legacy but avoid widening undefined leaks into public types.
## Dependencies and SCA
- **Major:** New runtime deps are rare—justify any addition; **`prepublishOnly`** and hooks assume **`npm test`** and Snyk-friendly trees.
- Use **`npm audit` / Snyk** expectations per org policy.
## Tests
- **Blocker:** Behavioral fixes or new branches without **`__test__`** coverage when risk is high (RTE nesting, GQL URL rewriting, endpoint resolution).
- **Minor:** Snapshot-only tests where a small assertion would be clearer.
## Terminology
- **Major:** Docs/comments must describe this as **utils** alongside **CDA / Delivery / JSON RTE / GraphQL**, not as **CMA** unless the change is explicitly management-related.