refactor: replace execSync with @actions/exec.getExecOutput#318
refactor: replace execSync with @actions/exec.getExecOutput#318
Conversation
|
Deploy preview for team-scope-test ready!
Deployed with vercel-action |
|
Deploy preview for zeit-now-deployment-action-example-angular ready!
Deployed with vercel-action |
|
Deploy preview for express-basic-auth ready!
Deployed with vercel-action |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the use of node:child_process's execSync with @actions/exec's getExecOutput in src/index.ts to unify command execution. This change involves converting getGitCommitMessage to an asynchronous function and updating its callers. Feedback suggests improving test coverage by adding a test case for the error handling path in getGitCommitMessage.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 8 files
Requires human review: This is a code refactor that changes a synchronous core utility function to an asynchronous one, which the instructions define as high-impact and requiring human review.
Architecture diagram
sequenceDiagram
participant Main as Action Main Loop
participant Context as getDeploymentContext()
participant GitMsg as getGitCommitMessage()
participant Exec as @actions/exec (Toolkit)
participant Git as Git CLI
Main->>Context: Initialize deployment context
Note over Context,GitMsg: Control flow is now asynchronous
Context->>GitMsg: CHANGED: await getGitCommitMessage()
GitMsg->>Exec: NEW: getExecOutput("git", ["log", "-1", ...])
rect rgb(240, 240, 240)
Note right of Exec: Replaces node:child_process.execSync
Exec->>Git: Spawn git process
Git-->>Exec: stdout: commit message
end
alt Execution Success
Exec-->>GitMsg: { stdout, exitCode: 0 }
GitMsg-->>Context: Return trimmed stdout string
else Execution Failure
Exec-->>GitMsg: Throw Error or non-zero exitCode
Note over GitMsg: Catch error and format string
GitMsg-->>Context: Return "Unknown (error message)"
end
Context-->>Main: Return DeploymentContext object
There was a problem hiding this comment.
Pull request overview
Refactors the Action’s git commit message retrieval to use the GitHub Actions toolkit exec utilities instead of Node’s child_process, aligning command execution patterns across the codebase and removing execSync usage.
Changes:
- Replaced
execSync('git log ...')with async@actions/exec.getExecOutput()ingetGitCommitMessage()and awaited it ingetDeploymentContext(). - Updated integration tests to mock
@actions/exec.getExecOutput()and removed thenode:child_processmock. - Added/registered a
.pleasetrack (spec/plan/metadata) documenting the work.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Switches git commit message collection to getExecOutput() and makes it async. |
| src/tests/index.test.ts | Updates mocks to support getExecOutput() instead of execSync. |
| dist/index.js | Rebuild output reflecting the new exec approach and removal of node:child_process. |
| .please/docs/tracks/index.md | Registers the new refactor track in the active tracks list. |
| .please/docs/tracks/active/use-actions-exec-20260329/spec.md | Adds track specification for the refactor. |
| .please/docs/tracks/active/use-actions-exec-20260329/plan.md | Adds implementation plan and verification notes for the refactor. |
| .please/docs/tracks/active/use-actions-exec-20260329/metadata.json | Adds track metadata (status, timestamps, issue link). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This is a code refactor that converts a core function from sync to async and modifies error handling logic. Per instructions, refactors and logic changes require human review.
amondnet
force-pushed
the
amondnet/best-countess
branch
from
c5373f7 to
d23b16c
Compare
Replace node:child_process execSync in getGitCommitMessage() with @actions/exec getExecOutput for consistent command execution across the codebase. No behavior change.
…ling - Use ignoreReturnCode: true in getExecOutput call and check exitCode explicitly, incorporating stderr into the thrown error for better diagnostics (copilot suggestion) - Add try/catch around getExecOutput call to wrap thrown errors with descriptive message (covers the throw-path for gemini/copilot suggestions) - Add tests for error path where getExecOutput throws (gemini suggestion) - Add tests for error path where getExecOutput returns non-zero exit code (copilot suggestion)
ncc 0.36.1 bundles TypeScript 4.3.4 which does not support ES2022 target/lib. Using --transpile-only skips ncc's internal type checking (handled separately by `pnpm typecheck`) and resolves the build failure.
amondnet
force-pushed
the
amondnet/best-countess
branch
from
d23b16c to
156631e
Compare
|
Summary
execSyncfromnode:child_processingetGitCommitMessage()with@actions/exec.getExecOutput()node:child_processdependency from production codeTest plan
pnpm test→ 117 passed)child_processimports insrc/(verified with grep)pnpm run build)Closes #317
Summary by cubic
Replaced
execSyncwith@actions/exec.getExecOutputto read the latest git commit message and removednode:child_processfrom production code. Behavior is unchanged; errors now include stderr/exit codes, tests cover error paths, and the build uses@vercel/nccwith--transpile-only; rebuiltdist/and addresses #317.getGitCommitMessage()async viagetExecOutputwith{ silent: true, ignoreReturnCode: true }, explicitexitCodechecks, and stderr in errors; awaited ingetDeploymentContext().getExecOutputand cover thrown and non-zero exit cases; added track docs under.please/docs/tracks/active/use-actions-exec-20260329/.--transpile-onlyto@vercel/nccto resolve TypeScript compatibility.Written for commit 156631e. Summary will update on new commits.
Verification Checklist
Automated Tests
pnpm test)node:child_processimports insrc/index.tsObservable Outcomes
grep -r 'child_process' src/returns no matches in production codepnpm testshows all tests passingpnpm run buildsucceeds without errorsAcceptance Criteria
node:child_processimports in production sourcegetGitCommitMessage()produces identical output