Skip to content

[jsweep] Clean messages_core.cjs - #20630

Merged
pelikhan merged 1 commit into
mainfrom
jsweep/clean-messages-core-66fc3221ee59a527
Mar 12, 2026
Merged

[jsweep] Clean messages_core.cjs#20630
pelikhan merged 1 commit into
mainfrom
jsweep/clean-messages-core-66fc3221ee59a527

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Summary

Cleaned actions/setup/js/messages_core.cjs and added a comprehensive test suite.

Context type: github-script (uses core global)


Changes

messages_core.cjs

  1. Moved lazy require() to top-level importerror_helpers.cjs was required inside getMessages() on every call. There's no circular dependency, so it belongs at the module top level like all other imports.

  2. Simplified toSnakeCase() with Object.fromEntries + flatMap — replaced the imperative for loop + two assignments with a functional one-liner. The new version also avoids duplicate entries when a key is already in snake_case (e.g. run_url previously got assigned twice):

    // Beforeconstresult={};for(const[key,value]ofObject.entries(obj)){constsnakeKey=key.replace(/([A-Z])/g,"_$1").toLowerCase();result[snakeKey]=value;result[key]=value;// duplicate if already snake_case}// AfterreturnObject.fromEntries(Object.entries(obj).flatMap(([key,value])=>{constsnakeKey=key.replace(/([A-Z])/g,"_$1").toLowerCase();returnsnakeKey===key ? [[key,value]] : [[snakeKey,value],[key,value]];}));

messages_core.test.cjs(new file)

Added 17 tests covering all three exported functions:

GroupTests
renderTemplatesingle placeholder, multiple placeholders, missing key preserved, undefined value preserved, numeric coercion, boolean coercion, no placeholders, empty template
toSnakeCasecamelCase→snake_case, original key preserved, no duplicates for snake_case input, multi-word conversion, empty object, mixed input
getMessagesnull when unset, null when empty, valid JSON parsed, multiple fields parsed, invalid JSON warns + returns null

Validation ✅

  • Formatting: npm run format:cjs
  • Linting: npm run lint:cjs
  • Type checking: npm run typecheck
  • Tests: npm run test:js ✓ (all 17 new tests pass; existing failures are pre-existing environment issues unrelated to this change)

Generated by jsweep - JavaScript Unbloater ·

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
allowed:
- defaults
- "proxy.golang.org"

See Network Configuration for more information.

  • expires on Mar 14, 2026, 3:31 AM UTC

- Move lazy require() of error_helpers.cjs to top-level import
- Simplify toSnakeCase() using Object.fromEntries + flatMap (avoids duplicate entries for already-snake_case keys)
- Add comprehensive test file messages_core.test.cjs with 17 tests covering renderTemplate, toSnakeCase, and getMessages
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
ContributorAuthor

🤖 Contribution Check — [jsweep] Clean messages_core.cjs

Nice work on this cleanup! The messages_core.cjs refactor is tight and well-reasoned — promoting the lazy require() to the module top level is the right call, and the flatMap/Object.fromEntries rewrite of toSnakeCase is cleaner while also fixing the latent duplicate-key bug for already-snake_case inputs. The 17-test suite covers all three exported functions with good boundary cases.

Checklist summary:

CheckResult
On-topic✅ yes — aligns with agentic JS cleanup tooling
Follows process✅ yes — agentic PR via core team automation
Focused✅ yes — single module refactor + tests
New dependencies✅ no
Has tests✅ yes — 17 tests across all exported functions
Has description✅ yes — thorough summary with before/after code
Lines changed198 (184 additions, 14 deletions)

Verdict: 🟢 Aligned — this PR looks ready for maintainer review!

Generated by Contribution Check ·

@pelikhan
pelikhan marked this pull request as ready for review March 12, 2026 13:48
CopilotAI review requested due to automatic review settings March 12, 2026 13:48

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up messages_core.cjs by hoisting a lazy require() to the module top level and simplifying toSnakeCase() using Object.fromEntries/flatMap, while also fixing a subtle duplicate-entry bug for already-snake_case keys. A comprehensive test suite is added.

Changes:

  • Moved require("./error_helpers.cjs") from inside getMessages() to the top of the module.
  • Rewrote toSnakeCase() as a functional one-liner that avoids duplicate entries for keys already in snake_case.
  • Added 17 new tests covering renderTemplate, toSnakeCase, and getMessages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

FileDescription
actions/setup/js/messages_core.cjsHoisted import; simplified toSnakeCase(); updated JSDoc comments.
actions/setup/js/messages_core.test.cjsNew test suite covering all three exported functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@pelikhan
pelikhan merged commit f151d2d into mainMar 12, 2026
4 checks passed
@pelikhan
pelikhan deleted the jsweep/clean-messages-core-66fc3221ee59a527 branch March 12, 2026 14:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pelikhan