Uh oh!
There was an error while loading. Please reload this page.
[code-simplifier] refactor(js): remove duplicate getDiscussionId, reuse getDiscussionNodeId - #42892
Conversation
…deId from add_reaction
add_reaction_and_edit_comment.cjs defined a local getDiscussionId(owner, repo, num)
that returned {id, url} but only id was ever used. The equivalent getDiscussionNodeId
is already exported from ./add_reaction.cjs, which was already imported. Remove the
31-line duplicate and import getDiscussionNodeId directly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (0 additions detected). |
✅ PR Code Quality Reviewer completed the code quality review. |
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Removes duplicate discussion node-ID lookup logic from actions/setup/js/add_reaction_and_edit_comment.cjs by reusing the already-exported getDiscussionNodeId helper from actions/setup/js/add_reaction.cjs, keeping discussion reaction/comment behavior the same while reducing duplicated GraphQL code.
Changes:
- Imports
getDiscussionNodeIdfrom./add_reaction.cjsand uses it for discussion node ID resolution. - Replaces prior
getDiscussionId(...).idusage with the direct node ID string returned bygetDiscussionNodeId(...). - Deletes the local
getDiscussionIdimplementation (previously returning{id, url}when onlyidwas used).
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/add_reaction_and_edit_comment.cjs | Reuses shared discussion node-ID helper and removes the duplicated local GraphQL lookup function. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Low
There was a problem hiding this comment.
Clean, behavior-preserving deduplication. Both call sites are correctly updated: discussion.id → direct string return, and { id: discussionId } destructuring → direct assignment. The removed url field from the old getDiscussionId return was dead (never consumed at any call site). Error handling, function signature, and module exports are all identical. No issues found.
🔎 Code quality review by PR Code Quality Reviewer · 24.5 AIC · ⌖ 6.96 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Review: Approved ✅
This is a clean, behavior-preserving refactor. Both call sites only consumed the .id field from the old getDiscussionId({id, url}) return value — the .url field was fetched but never used. The imported getDiscussionNodeId has an identical signature and error-handling path, making this a safe deduplication.
Verified:
- Both original call sites only used
.id(viadiscussion.idand{ id: discussionId }destructuring) getDiscussionNodeIdinadd_reaction.cjshas the same signature(owner, repo, num), the same GraphQL query structure, and the sameERR_NOT_FOUNDerror throwmodule.exportsof the modified file is unchanged- Net result: 31 lines removed, no behavior change
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 19.9 AIC · ⌖ 6.92 AIC · ⊞ 4.9K
There was a problem hiding this comment.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 53.6 AIC · ⌖ 9.56 AIC · ⊞ 6.6K
Comment /matt to run again
gh-aw-bot
commented
Jul 2, 2026
@copilot please run the
|
Done. Here's the summary:
Actions taken: No code changes needed. The branch was already up to date, all checks were green, and there were no unresolved review comments to address. Hand-off: CI must be re-triggered by a maintainer (close/reopen PR, |
Uh oh!
There was an error while loading. Please reload this page.
🎉 This pull request is included in a new release. Release: |
Caution
agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.
Details
Potential security threats were detected in the agent output.
Review the workflow run logs for details.
test body