[jsweep] Clean substitute_placeholders.cjs#19315
Conversation
- Add @ts-check and /// <reference types="@actions/github-script" /> for type safety - Add JSDoc type annotations for function parameters and return type - Extract log() helper to eliminate 20+ repeated typeof core checks - Fix yoda condition: "object" != typeof → typeof !== "object" - Simplify null check: value === undefined || value === null → value == null - Refactor test file: add vitest imports, remove comma-operator anti-pattern Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
…rs.cjs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
There was a problem hiding this comment.
Pull request overview
Cleans up the substitute_placeholders helper and its test suite as part of the jsweep unbloat effort, improving type-safety and reducing verbose logging.
Changes:
- Added
// @ts-check+@actions/github-scripttype references and JSDoc typing forsubstitutePlaceholders. - Ensured
coreis always available by requiringshim.cjs, and reduced logging to a single summary line per invocation. - Refactored the test file to use standard Vitest imports and explicit
it()blocks (removing the comma-operator chaining).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/substitute_placeholders.cjs | Adds TS checking/JSDoc types, uses shim-provided core, and simplifies logging + null handling. |
| actions/setup/js/substitute_placeholders.test.cjs | Refactors tests into conventional Vitest structure and adds basic JSDoc types for local variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Commit pushed:
|
Smoke Test Results — Run §22607233572 | Status: PARTIAL
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! The cleanup of substitute_placeholders.cjs is clean and well-executed. The removal of verbose conditional core.info guards in favor of the shim.cjs approach reduces code by ~60 lines while maintaining functionality. Test structure improvements in substitute_placeholders.test.cjs are also a significant readability win.
💥 [THE END] — Illustrated by Smoke Claude
Cleaned
actions/setup/js/substitute_placeholders.cjsas part of the jsweep unbloat initiative.Context type: github-script (uses
core.info()for logging)Changes
substitute_placeholders.cjs@ts-checkwith/// (reference types="@actions/github-script" /)for full type safetyfile: string,substitutions: Record(string, string | null | undefined)) and return type (Promise(string))shim.cjs— ensurescoreis always available as a global even when running outside github-script context, replacing thetypeof core !== "undefined"guard patternlog()helper — usescore.info()directly (guaranteed by shim), with logging reduced to a single summary line per invocation instead of 20+ verbose diagnostic lines"object" != typeof substitutions→typeof substitutions !== "object"value === undefined || value === null ? ""→value == null ? ""(idiomatic JS)substitute_placeholders.test.cjsimport { afterEach, beforeEach, describe, expect, it } from "vitest"(matches codebase pattern)it()calls instead of comma-chained expressionsTest Coverage
All 12 existing tests pass with no regressions. Tests cover:
undefined/nullvalues treated as empty stringsValidation ✅
All checks passed:
npm run format:cjs✓npm run lint:cjs✓npm run typecheck✓npm run test:js -- --no-file-parallelism substitute_placeholders✓ (12/12 passed)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
[jsweep] Clean substitute_placeholders.cjs
Cleaned
actions/setup/js/substitute_placeholders.cjsas part of the jsweep unbloat initiative.Context type: github-script (uses
core.info()for logging)Changes
substitute_placeholders.cjs@ts-checkwith/// (reference types="@actions/github-script" /)for full type safetyfile: string,substitutions: Record(string, string | null | undefined)) and return type (Promise(string))shim.cjs— ensurescoreis always available as a global even when running outside github-script context, replacing thetypeof core !== "undefined"guard patternlog()helper — usescore.info()directly (guaranteed by shim), with logging reduced to a single summary line per invocation instead of 20+ verbose diagnostic lines"object" != typeof substitutions→typeof substitutions !== "object"value === undefined || value === null ? ""→value == null ? ""(idiomatic JS)substitute_placeholders.test.cjsimport { afterEach, beforeEach, describe, expect, it } from "vitest"(matches codebase pattern)it()calls instead of comma-chained expressionsTest Coverage
All 12 existing tests pass with no regressions. Tests cover:
undefined/nullvalues treated as empty stringsValidation ✅
All checks passed:
npm run format:cjs✓npm run lint:cjs✓npm run typecheck✓npm run test:js -- --no-file-parallelism substitute_placeholders✓ (12/12 passed)✨ Let Copilot coding agent [set things up for you](https://github.com/github/gh-aw/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repo%5D(gh.io/redacted) — coding agent works faster and does higher quality work when set up for your repo.
Changeset
Warning
The following domains were blocked by the firewall during workflow execution:
codeload.github.comgithub.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
✨ PR Review Safe Output Test - Run 22607233572