|
| 1 | +# Commit review — 2026-04-18 |
| 2 | + |
| 3 | +## Window |
| 4 | + |
| 5 | +- Start: 2026-04-17 23:56:33 -0700 |
| 6 | +- End: 2026-04-18 04:27:51 -0700 |
| 7 | +- SHA range: `3af178cc07..414d61b8ad` |
| 8 | +- Commits: 34 (first-parent into `develop`) |
| 9 | +- Files changed: 210 |
| 10 | +- Union diff: 15,381 lines |
| 11 | + |
| 12 | +## PR |
| 13 | + |
| 14 | +- Branch: `philipp/fix-commit-review-2026-04-18` |
| 15 | +- URL: https://github.com/stdlib-js/stdlib/pull/11556 |
| 16 | +- State: draft |
| 17 | + |
| 18 | +## Validated issues and resolutions |
| 19 | + |
| 20 | +### `napi/create-int64` (commit `0c116e3d`) |
| 21 | + |
| 22 | +1. **`lib/node_modules/@stdlib/napi/create-int64/src/main.c`** — `@example` block referenced an undeclared `err` variable copy-pasted from `argv-int64`; `stdlib_napi_create_int64` has no `err` parameter. |
| 23 | + - Resolution: removed the `if ( err != NULL ) { ... }` block. |
| 24 | +2. **`lib/node_modules/@stdlib/napi/create-int64/README.md`** — C API Usage example had the same spurious `err` / `napi_throw` block. |
| 25 | + - Resolution: removed the block. |
| 26 | +3. **`lib/node_modules/@stdlib/napi/create-int64/test/test.native.js`** — `strictEqual( v, values[i] )` compared a BigInt (returned by `napi_create_bigint_int64`) against a JS Number; fails whenever the native addon builds. |
| 27 | + - Resolution: added `var Number = require( '@stdlib/number/ctor' );` and changed the assertion to `t.strictEqual( Number( v ), values[ i ], ... )`. |
| 28 | + |
| 29 | +### `napi/create-uint64` (commit `0c116e3d`) |
| 30 | + |
| 31 | +4. **`lib/node_modules/@stdlib/napi/create-uint64/src/main.c`** — same `err` copy-paste in the `@example` block. |
| 32 | + - Resolution: removed the block. |
| 33 | +5. **`lib/node_modules/@stdlib/napi/create-uint64/README.md`** — same `err` copy-paste in the C API Usage example. |
| 34 | + - Resolution: removed the block. |
| 35 | +6. **`lib/node_modules/@stdlib/napi/create-uint64/README.md`** — C API Notes section was empty; sibling `create-int64` README documents that the generated JavaScript value is a `BigInt` (N-API Version 6+). |
| 36 | + - Resolution: added the matching note. |
| 37 | +7. **`lib/node_modules/@stdlib/napi/create-uint64/test/test.native.js`** — same BigInt-vs-Number `strictEqual` mismatch. |
| 38 | + - Resolution: added the `@stdlib/number/ctor` import and wrapped the actual value with `Number()`. |
| 39 | + |
| 40 | +### `ndarray/colcat` (commit `648e90d0`) |
| 41 | + |
| 42 | +8. **`lib/node_modules/@stdlib/ndarray/colcat/README.md`** — Examples code block missing the `<!-- eslint no-undef: "error" -->` directive used by every other `@stdlib/ndarray` top-level README. |
| 43 | + - Resolution: inserted the directive between `## Examples` and the opening code fence. |
| 44 | + |
| 45 | +## Fix commits |
| 46 | + |
| 47 | +- `480bda57b` — fix: drop undeclared err check and compare BigInt via Number in napi/create-int64 |
| 48 | +- `cfe45e8f7` — fix: drop undeclared err check, document BigInt return, and compare via Number in napi/create-uint64 |
| 49 | +- `3b18bd65c` — style: add missing eslint no-undef directive to ndarray/colcat Examples block |
| 50 | + |
| 51 | +## Issues considered and dropped |
| 52 | + |
| 53 | +| Issue | Source | Reason dropped | |
| 54 | +| --- | --- | --- | |
| 55 | +| Commit-message typo `dcumxabs` → `dcumaxabs` (commit `adc6eb6b`) | style agent 2 | Cannot fix on a merged commit without rewriting `develop` history. | |
| 56 | +| Commit-message typo `daxpy` → `zaxpy` (commit `b0c2f7b7`) | style agent 2 | Same — merged commit-message rewrite is out of scope for a fix PR. | |
| 57 | +| `stats/base/ndarray/svariance/README.md` commented-out `<img>` tags point to `stats/strided/svariance/...` paths | style agent 1 | Verified intentional: the sibling `stats/base/ndarray/variance/README.md` uses the same pattern (references the strided package's docs images). | |
| 58 | +| `napi/argv-uint64/src/main.c` JS-number path reads `int64` then casts to `uint64`, so negative numbers silently wrap to large unsigned values | bug agent 4 | Required interpretation — test file only exercises non-negative values and the summary did not label this as a deliberate choice; without explicit confirmation, the fix (reject negatives) is a judgment call. | |
| 59 | + |
| 60 | +## Reviewer configuration |
| 61 | + |
| 62 | +- Summary agent: sonnet, read union diff + commit log, produced author-intent summary for downstream reviewers. |
| 63 | +- Reviewer 1 (style, new-package structure): sonnet, new packages under `napi/*`, `blas/base/ndarray/*axpy`, `ndarray/base/{tile,trues,falses}`, `ndarray/colcat`, `stats/base/ndarray/{svariance,dnanrangeabs}`. |
| 64 | +- Reviewer 2 (style, doctests / C / namespace): sonnet, collapsed doctests across `stats/base/ndarray/cum*`, namespace additions, `napi/argv-*` BigInt changes, README copy updates. |
| 65 | +- Reviewer 3 (bug, diff-only): opus. |
| 66 | +- Reviewer 4 (bug, logic/security): opus, allowed to Read files listed in the changed-files set. |
0 commit comments