Skip to content

Commit 58d1616

Browse files
GMShannon99claude
andcommitted
Document self-review findings in NOTES.md
Recorded what the pre-PR review caught (truthy-only validation, duplicated id/validation/404 blocks) and why they were left as-is — they match this repo's existing conventions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0b7d9c9 commit 58d1616

1 file changed

Lines changed: 11 additions & 0 deletions

File tree

NOTES.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,14 @@ Checked the three grading tests in `tests/update-user.test.js` line by line agai
3333
handler: update-and-200, unknown-id-404, and missing-field-400. Confirmed the existing
3434
`tests/users.test.js` and `tests/notes.test.js` still pass, and that `npm run lint` stays
3535
clean (no new eslint-disable comments needed, no unused vars).
36+
37+
Ran a broader self-review (correctness, reuse, and cross-file angles) before opening the
38+
PR. It confirmed the not-found and malformed-JSON paths already behave the same as the
39+
existing `GET /users/:id` and `POST /users` routes (nothing new introduced there), and
40+
flagged two things worth naming rather than fixing: (1) validation is truthy-only, so a
41+
value like `name: 0` would pass and get persisted — an existing gap in `POST /users` that
42+
this change now also applies to overwriting existing records; and (2) the id-parsing,
43+
validation, and 404-response blocks are each a few lines duplicated from `GET /:id` /
44+
`POST /`. Left both as-is: they match this repo's existing (helper-free) conventions, and
45+
extracting shared validation for two fields across a 53-line file would be inconsistent
46+
with the rest of the codebase.

0 commit comments

Comments
 (0)