Refactor/clean-up diff-informed utils - #4300
mbg wants to merge 5 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
🔵 Needs a closer look
The lockfile still resolves glob to 13.0.6 despite the workspace requiring ^14.0.3, leaving a dependency mismatch that may cause npm ci to fail.
No open issues 🎉
What changed in this PRThis PR consolidates diff-informed analysis utilities into a dedicated module and makes persisted diff-range paths explicit, enabling tests to rely less on global environment state.
Changes:
- Moves utilities into src/diff-informed and expands their JSDocs.
- Updates callers and tests for explicit file-path arguments.
- Refreshes the generated JavaScript bundle and updates the workspace’s glob requirement metadata.
| File | Description |
|---|---|
| src/upload-lib.ts | Passes an explicit path when reading diff ranges. |
| src/upload-lib.test.ts | Updates imports and stubs the path helper. |
| src/overlay/index.ts | Uses the relocated path helper. |
| src/overlay/index.test.ts | Updates path-helper stubs. |
| src/diff-informed/index.ts | Consolidates utilities, adds path parameters, and improves JSDocs. |
| src/diff-informed/index.test.ts | Relocates tests and updates function arguments. |
| src/config-utils.ts | Computes the diff-range path from action state. |
| src/config-utils.test.ts | Configures temporary paths through test environments. |
| src/analyze.ts | Uses the new module and explicit read path. |
| src/actions-util.ts | Removes the relocated path helper. |
| package-lock.json | Updates workspace glob requirement metadata. |
| lib/entry-points.js | Generated bundle reflecting the TypeScript refactor. |
Review effort: Balanced (auto)
Sorry, something went wrong.
There was a problem hiding this comment.
🟡 Changes recommended
The test setup leaves hosted-runner detection dependent on the real CI environment, which can unexpectedly enable TRAP caching and break assertions expecting zero download time.
1 open issueReview effort: Max
Sorry, something went wrong.
| const languages = "javascript,python"; | ||
|
|
||
| setupActionsVars(tempDir, tempDir); | ||
| setupActionsVars(tempDir, tempDir, {}, state.env); |
This PR refactors diff-informed analysis utilities into a new diff-informed subfolder, improves the JSDocs for the functions, and makes the file path an explicit argument rather than implicitly computed value based on an environment variable so that tests don't have to rely on global state manipulation going forward.