Conversation
- Remove unnecessary `const item = message` alias, use `message` directly - Remove redundant discussion ID pre-check before commentOnDiscussion (saves 1-2 API calls per discussion comment; commentOnDiscussion already checks internally) - Extract `recordComment` helper to eliminate duplicated comment-result construction - Remove unused `result` variable in hideOlderComments loop - Update test: check for 'Created comment on discussion' log instead of removed 'Found discussion' log Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot recompile, run tests |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — recompiled 173/173 workflows and all 56 JS tests pass (14c7b1b). Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Cleans up add_comment.cjs by removing a redundant variable alias, eliminating duplicate discussion-existence pre-checks (saving API calls), extracting a recordComment helper, and removing an unused variable assignment. Lock files are also regenerated, picking up an unrelated close_pull_request tool.
Changes:
- Removed
const item = messagealias and redundant GraphQL discussion pre-checks, reducing code by ~82 lines - Extracted
recordCommenthelper to deduplicate success-result construction - Updated one test assertion to match a changed log message
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/add_comment.cjs | Core cleanup: remove alias, pre-checks, unused var; add recordComment helper |
| actions/setup/js/add_comment.test.cjs | Update assertion to check for remaining log message |
| .github/workflows/smoke-claude.lock.yml | Lockfile regen; adds close_pull_request config |
| .github/workflows/poem-bot.lock.yml | Lockfile regen; adds close_pull_request config |
| .github/workflows/draft-pr-cleanup.lock.yml | Lockfile regen; adds close_pull_request config |
💡 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.
| mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs | ||
| cat > /opt/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' | ||
| {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} | ||
| {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} |
Summary
Cleaned
actions/setup/js/add_comment.cjs— a github-script context handler for adding comments to issues, PRs, and discussions.Changes
add_comment.cjs(-82 lines net)1. Remove
const item = messagealiasThe handler was declaring
const item = messageimmediately on entry and usingitemthroughout. Removed the alias and usedmessagedirectly — eliminates a confusing layer of indirection.2. Remove redundant discussion ID pre-checks (saves 1–2 API calls per run)
In two places (main discussion path and 404-fallback retry path), the code was querying GitHub to verify a discussion existed before calling
commentOnDiscussion. ButcommentOnDiscussionalready does the same internal query and throws the sameERR_NOT_FOUNDerror if not found. Removing the pre-checks reduces API call count with no behavioral change:3. Extract
recordCommenthelperThe success-result construction (building the
_trackingobject, pushing tocreatedComments, returning the result) was duplicated in both the main path and the discussion fallback path. Extracted into a localrecordComment(comment, isDiscussionFlag)helper.4. Remove unused
resultvariable inhideOlderCommentsconst result = await minimizeComment(...)—resultwas assigned but never read. Removed the assignment.add_comment.test.cjs(1 line changed)Updated one test assertion that checked for the now-removed log message
"Found discussion #X, adding comment...". Changed to check for"Created comment on discussion"which is still logged after a successful discussion comment.Test Coverage
56 tests — all passing. The file already had comprehensive coverage; no new tests needed.
Validation ✅
npm run format:cjs✓ (unchanged)npm run lint:cjs✓ (all files pass Prettier)npm run typecheck✓ (no errors)npm run test:js add_comment.test.cjs✓ (56/56 passed)Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.