Conversation
- Remove redundant '&& targetStr != ""' check (setting Target to empty string is equivalent to not setting it; consistent with create_pr_review_comment.go) - Remove per-target debug log (not present in other similar parsers) - Move repo/repoParts declarations inside the else-if branch where they are used, narrowing scope and improving readability Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pelikhan
approved these changes
Feb 18, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies recently added code from PR #16602 to align with established patterns in the codebase. The changes remove redundant code and improve variable scoping without altering functionality.
Changes:
- Simplified target parsing in Go to match the pattern used in
create_pr_review_comment.go - Narrowed variable scope in JavaScript for better code clarity
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/workflow/submit_pr_review.go | Removed redundant empty string check and debug log from target parsing to match create_pr_review_comment.go pattern |
| actions/setup/js/submit_pr_review.cjs | Moved repo and repoParts variable declarations inside the else if (targetResult.number) block where they're actually used |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Simplifies recently added code in
submit_pr_review.goandsubmit_pr_review.cjs(from PR #16602) for consistency with existing patterns in the codebase.Files Simplified
pkg/workflow/submit_pr_review.go— Remove redundant guard and debug log in target parsingactions/setup/js/submit_pr_review.cjs— Move variable declarations closer to where they're usedImprovements Made
1. Consistent Target Parsing (Go)
The newly added target parsing in
submit_pr_review.goincluded an extra&& targetStr != ""guard and a debug log not present in the equivalentcreate_pr_review_comment.go:Before:
After (matching
create_pr_review_comment.go):The
!= ""check is redundant — settingTargetto""is equivalent to not setting it (empty string is the zero value and the validation layer treats it as the default "triggering" behavior). The extra log line was inconsistent with how other parsers handle target.2. Narrowed Variable Scope (JavaScript)
repoandrepoPartswere declared before theresolveTarget()call but only used inside theelse if (targetResult.number)branch. Moving them inside that branch narrows their scope, making the code easier to follow.Based On
PR #16602 — Fix submit_pull_request_review when not in pull_request trigger
Testing
node --check)TestSubmitPullRequestReviewConfigpasses"target": "42"and verifies correct parsingReferences: §22153377350