Conversation
- Extract renderConfiguredMessage() helper to eliminate 7x repeated pattern (getMessages + toSnakeCase + ternary renderTemplate call) - Remove unused /// <reference types="@actions/github-script" /> directive (module uses no github-script globals) - Each exported function is now a readable one-liner - Add comprehensive test file with 20 test cases covering all 7 functions, default templates, custom templates, placeholder substitution, camelCase/snake_case keys, and fallback behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the run-status message generator module to remove duplicated “configured vs default template” logic and adds dedicated unit tests for the run-status message functions.
Changes:
- Extracted a shared
renderConfiguredMessage()helper to centralize message rendering with optional config overrides. - Removed an unused
@actions/github-scriptreference directive frommessages_run_status.cjs. - Added
messages_run_status.test.cjswith coverage for defaults, config overrides, and placeholder substitution behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| actions/setup/js/messages_run_status.cjs | Refactors message generation to use a shared helper for default/configured template rendering. |
| actions/setup/js/messages_run_status.test.cjs | Adds Vitest coverage for all exported run-status message helpers and template substitution cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| function renderConfiguredMessage(messageKey, defaultTemplate, ctx) { | ||
| const messages = getMessages(); | ||
| const template = messages?.[messageKey] ?? defaultTemplate; |
There was a problem hiding this comment.
renderConfiguredMessage uses nullish-coalescing (messages?.[messageKey] ?? defaultTemplate). This changes behavior vs the previous truthy check: an empty-string configured message will now override the default, and a non-string (e.g. false, 0) would be passed into renderTemplate and throw at runtime (template.replace is not a function). Consider preserving the prior semantics and adding a type guard, e.g. only use the configured value when it is a non-empty string; otherwise fall back to defaultTemplate (and optionally emit a warning for invalid types).
| const template = messages?.[messageKey] ?? defaultTemplate; | |
| const configuredTemplate = messages && messages[messageKey]; | |
| const template = typeof configuredTemplate === "string" && configuredTemplate | |
| ? configuredTemplate | |
| : defaultTemplate; |
|
@copilot review comments |
Applied the suggested type guard in |
Summary
Cleaned
actions/setup/js/messages_run_status.cjs— a Node.js context module (no github-script globals) that provides run-status message generation for workflow execution notifications.Changes
messages_run_status.cjsExtracted
renderConfiguredMessage()helper to eliminate a 7× repeated pattern:Also removed the unused
/// <reference types="@actions/github-script" />directive — this module only usesrequire(), nocore/github/contextglobals.messages_run_status.test.cjs(new file)Added 20 test cases covering all 7 exported functions:
GH_AW_SAFE_OUTPUT_MESSAGESenv vargetRunFailureMessagegetCommitPushedMessageValidation ✅
npm run format:cjsnpm run lint:cjsnpm run typechecknpm run test:js