Conversation
Deleted unreachable functions from utility packages: - pkg/sliceutil: ContainsAny, ContainsIgnoreCase (+ all tests) - pkg/logger: NewSlogLogger, Discard (+ tests for NewSlogLogger) - pkg/fileutil: CalculateDirectorySize (+ test) - pkg/repoutil: ParseGitHubURL (+ all tests) - pkg/stringutil/identifiers: IsAgenticWorkflow, IsLockFile (+ tests) - pkg/stringutil/pat_validation: IsFineGrainedPAT, IsClassicPAT, IsOAuthToken (+ tests) - pkg/stringutil/stringutil: StripANSIEscapeCodes (+ tests) Also replaces sliceutil.ContainsAny calls in docker_images_test.go with inline strings.Contains expressions. 986 lines removed across 15 files.
- Delete EnsureLocalhostDomains from pkg/parser/mcp.go and its test - Delete AddIfTrue from pkg/workflow/compiler_safe_outputs_config.go - Delete HasUserCheckouts from pkg/workflow/checkout_manager.go - Delete 14 dead methods from pkg/workflow/artifact_manager.go (SetCurrentJob, GetCurrentJob, RecordUpload, RecordDownload, computeNormalizedPaths, findCommonParent, ComputeDownloadPath, FindUploadedArtifact, ValidateDownload, matchesPattern, GetUploadsForJob, GetDownloadsForJob, ValidateAllDownloads, GetAllArtifacts) - Delete artifact_manager test files (all tests tested deleted methods): artifact_manager_test.go, artifact_manager_integration_test.go, artifact_manager_workflows_integration_test.go 2216 lines removed across 9 files
Delete dead parser functions and their exclusive tests: pkg/parser/include_processor.go: - Delete ProcessIncludes pkg/parser/include_expander.go: - Delete ExpandIncludes, ProcessIncludesForEngines, ProcessIncludesForSafeOutputs pkg/parser/schema_validation.go: - Delete ValidateMainWorkflowFrontmatterWithSchema - Delete ValidateIncludedFileFrontmatterWithSchema - Delete ValidateMCPConfigWithSchema pkg/parser/yaml_error.go: - Delete ExtractYAMLError, extractFromGoccyFormat, extractFromStringParsing Test cleanup (deleted whole files): - schema_oneof_test.go - schema_passthrough_validation_test.go - schema_additional_properties_test.go - frontmatter_syntax_errors_test.go - schema_validation_test.go Test cleanup (removed specific functions): - frontmatter_includes_test.go: 5 functions for ProcessIncludes/ExpandIncludes - schema_test.go: 3 functions for dead schema validators - schema_utilities_test.go: 2 functions for dead validators - yaml_error_test.go: 3 functions for ExtractYAMLError - import_syntax_test.go: TestProcessIncludesWithNewSyntax - frontmatter_benchmark_test.go: 2 benchmark functions 5129 lines removed across 15 files
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes dead/unused JavaScript “getter” stub functions from pkg/workflow/js.go and replaces their remaining call sites with inline empty script strings, consistent with the current “scripts provided at runtime” model.
Changes:
- Deleted unused
get*Script()stub functions that only returned"". - Replaced several
Script: get*Script()assignments withScript: "". - Kept only the small set of remaining script stub getters still referenced elsewhere.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/js.go | Removes multiple unused script getter stubs; keeps a small remaining subset. |
| pkg/workflow/create_pr_review_comment.go | Replaces script getter call with inline empty script. |
| pkg/workflow/create_discussion.go | Replaces script getter call with inline empty script. |
| pkg/workflow/create_code_scanning_alert.go | Replaces script getter call with inline empty script. |
| pkg/workflow/compiler_safe_outputs_specialized.go | Replaces create-project script getter call with inline empty script. |
| pkg/workflow/add_comment.go | Replaces script getter call with inline empty script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
68
to
+73
| // All getter functions return empty strings since embedded scripts were removed | ||
|
|
||
| func getAddCommentScript() string { return "" } | ||
| func getAssignToAgentScript() string { return "" } | ||
| func getCreateCodeScanningAlertScript() string { return "" } | ||
| func getCreateDiscussionScript() string { return "" } | ||
| func getCreateIssueScript() string { return "" } | ||
|
|
||
| //nolint:unused // Only used in integration tests | ||
| func getCreatePRReviewCommentScript() string { return "" } | ||
| func getNoOpScript() string { return "" } | ||
| func getNotifyCommentErrorScript() string { return "" } | ||
| func getCreateProjectScript() string { return "" } | ||
| func getUploadAssetsScript() string { return "" } | ||
| func getAssignToAgentScript() string { return "" } | ||
| func getNoOpScript() string { return "" } | ||
| func getNotifyCommentErrorScript() string { return "" } | ||
| func getUploadAssetsScript() string { return "" } |
There was a problem hiding this comment.
The comment All getter functions return empty strings... is now inaccurate/misleading: this file still has getters like GetLogParserScript returning a non-empty placeholder, and only a small subset of private get*Script() stubs remain below. Suggest rewording to something more precise (e.g., specifically about the remaining private get*Script() stubs) to avoid confusion for future maintainers.
See below for a potential fix:
// Private get*Script stubs return empty strings now that embedded scripts were removed.
func getAssignToAgentScript() string { return "" }
func getNoOpScript() string { return "" }
func getNotifyCommentErrorScript() string { return "" }
func getUploadAssetsScript() string { return "" }
// Public Get* functions no longer return embedded script contents; most return empty
// strings or placeholders because scripts are provided at runtime instead.
This was referenced Feb 28, 2026
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
get*Script()stub functions fromjs.goand replaces their call sites with inline empty strings ("")parser,workflow, and 7 other packagesDEADCODE.mddocumenting aconsole_wasm.gopitfall