[dead-code] chore: remove dead functions — 8 functions removed#19750
Merged
[dead-code] chore: remove dead functions — 8 functions removed#19750
Conversation
Removes unreachable Go functions identified by the deadcode static analyzer. Functions removed: - FormatAggregatedError (pkg/workflow/error_aggregation.go) - SplitJoinedErrors (pkg/workflow/error_aggregation.go) - ValidateExpressionSafetyPublic (pkg/workflow/expression_validation.go) - extractMapFromFrontmatter (pkg/workflow/frontmatter_extraction_metadata.go) - unmarshalFromMap (pkg/workflow/frontmatter_types.go) - GetCurrentGitTag (pkg/workflow/git_helpers.go) - RunGit (pkg/workflow/git_helpers.go) - ExecGHWithOutput (pkg/workflow/github_cli.go) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Removes dead/unreachable workflow helper functions (and their dedicated tests) as identified by static analysis, reducing maintenance surface area.
Changes:
- Deleted unused helper functions across error aggregation, expression validation, frontmatter parsing/extraction, git helpers, and GitHub CLI helpers.
- Removed/updated tests that only exercised the deleted functions and cleaned up now-unused imports.
- Adjusted frontmatter backward-compatibility test to call
ExtractMapFielddirectly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/tools_types_test.go | Removes test that only covered the deleted extractMapFromFrontmatter wrapper. |
| pkg/workflow/imports_inputs_test.go | Removes test that only covered the deleted ValidateExpressionSafetyPublic wrapper. |
| pkg/workflow/github_cli.go | Drops ExecGHWithOutput and removes now-unused go-gh/v2/bytes imports. |
| pkg/workflow/git_helpers_test.go | Removes tests that only covered deleted GetCurrentGitTag. |
| pkg/workflow/git_helpers.go | Removes GetCurrentGitTag and RunGit plus now-unused os import. |
| pkg/workflow/frontmatter_types_test.go | Removes tests that only covered deleted unmarshalFromMap; updates backward-compat test to use ExtractMapField. |
| pkg/workflow/frontmatter_types.go | Removes unused unmarshalFromMap helper. |
| pkg/workflow/frontmatter_extraction_metadata.go | Removes unused extractMapFromFrontmatter wrapper. |
| pkg/workflow/expression_validation.go | Removes unused ValidateExpressionSafetyPublic wrapper. |
| pkg/workflow/error_aggregation_test.go | Removes tests/imports that only covered deleted FormatAggregatedError / SplitJoinedErrors. |
| pkg/workflow/error_aggregation.go | Removes unused FormatAggregatedError / SplitJoinedErrors helpers. |
Comments suppressed due to low confidence (1)
pkg/workflow/github_cli.go:13
- The go-gh/v2 dependency was removed from this file, but there are still log messages and doc comments later in the file that claim commands are executed "via go-gh/v2". This is now misleading since the implementation uses os/exec directly; please update the wording (and/or remove the token-based branching language) to match the current behavior.
import (
"context"
"os"
"os/exec"
"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/tty"
)
💡 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.
Comment on lines
136
to
140
| sb.WriteString(err.Error()) | ||
| } | ||
|
|
||
| return fmt.Errorf("%s", sb.String()) | ||
| } |
There was a problem hiding this comment.
The FormattedError doc comment still references FormatAggregatedError ("preferred over Error() + FormatAggregatedError"), but FormatAggregatedError was removed in this change. Please update the comment to avoid referring to a non-existent helper (e.g., just compare against Error()).
Comment on lines
37
to
41
| package workflow | ||
|
|
||
| import ( | ||
| "os" | ||
| "os/exec" | ||
| "strings" |
There was a problem hiding this comment.
The file header comment ("# Key Functions") still lists GetCurrentGitTag() and RunGit() even though both were removed in this PR. Please update that section to reflect the remaining public API (e.g., RunGitCombined) so the module-level documentation stays accurate.
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.
Dead Code Removal
This PR removes unreachable Go functions identified by the
deadcodestatic analyzer.Functions Removed
FormatAggregatedErrorpkg/workflow/error_aggregation.goSplitJoinedErrorspkg/workflow/error_aggregation.goValidateExpressionSafetyPublicpkg/workflow/expression_validation.goextractMapFromFrontmatterpkg/workflow/frontmatter_extraction_metadata.gounmarshalFromMappkg/workflow/frontmatter_types.goGetCurrentGitTagpkg/workflow/git_helpers.goRunGitpkg/workflow/git_helpers.goExecGHWithOutputpkg/workflow/github_cli.goTests Removed
TestFormatAggregatedError_NoError,TestFormatAggregatedError_SingleError,TestFormatAggregatedError_MultipleErrors— exclusively testedFormatAggregatedErrorTestSplitJoinedErrors_NoError,TestSplitJoinedErrors_SingleError,TestSplitJoinedErrors_MultipleErrors— exclusively testedSplitJoinedErrorsTestImportInputsExpressionValidation— exclusively testedValidateExpressionSafetyPublicTestExtractMapFromFrontmatter— exclusively testedextractMapFromFrontmatterTestUnmarshalFromMap— exclusively testedunmarshalFromMapTestGetCurrentGitTag(entiregit_helpers_test.go) — exclusively testedGetCurrentGitTagAlso updated
TestFrontmatterConfigBackwardCompatibilityto useExtractMapFielddirectly (the underlying functionextractMapFromFrontmatterwas just a thin wrapper around it).Also removed now-unused imports:
osfromgit_helpers.go,bytesandgithub.com/cli/go-gh/v2fromgithub_cli.go,stringsfromerror_aggregation_test.go.Verification
go build ./...— passesgo vet ./...— passesgo vet -tags=integration ./...— passesmake fmt— no changes neededgo test ./pkg/workflow/...— passes (27.8s)Dead Function Count
Automated by Dead Code Removal workflow — https://github.com/github/gh-aw/actions/runs/22730774949