Conversation
Remove unreachable functions identified by the deadcode static analyzer: - GetActionPins (pkg/actionpins): public wrapper, dead since only callers are tests and the dead getActionPins wrapper in pkg/workflow - FetchWorkflowFromSource (pkg/cli): thin wrapper around FetchWorkflowFromSourceWithContext; only test callers - getActionPins (pkg/workflow): package-private wrapper around actionpins.GetActionPins; only test callers - buildConsolidatedSafeOutputStep (Compiler method, pkg/workflow): only test callers Also remove exclusive tests for each deleted function, and update non-exclusive tests in spec_test.go that used GetActionPins() as a helper to discover a known repo (replaced with hardcoded 'actions/checkout'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Great work from the Dead Code Removal Agent! 🤖 This PR cleanly removes 4 unreachable functions ( This looks ready for maintainer review. ✅
|
|
@copilot recompile |
There was a problem hiding this comment.
Pull request overview
Removes functions flagged as unreachable by the deadcode analyzer, along with tests that exclusively covered those functions, and updates remaining tests accordingly.
Changes:
- Removed dead/unused functions in
pkg/actionpins,pkg/cli, andpkg/workflow. - Deleted tests that only validated the removed functions.
- Updated
pkg/actionpins/spec_test.goto use a stable known repo (actions/checkout) instead of calling the deleted API.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_safe_outputs_steps.go | Removes Compiler.buildConsolidatedSafeOutputStep implementation. |
| pkg/workflow/compiler_safe_outputs_steps_test.go | Removes tests that only exercised buildConsolidatedSafeOutputStep. |
| pkg/workflow/action_pins.go | Removes package-private getActionPins test wrapper. |
| pkg/workflow/action_pins_test.go | Removes tests that only exercised the deleted getActionPins wrapper. |
| pkg/cli/fetch.go | Removes exported FetchWorkflowFromSource wrapper. |
| pkg/cli/remote_workflow_test.go | Removes tests that only exercised FetchWorkflowFromSource. |
| pkg/actionpins/actionpins.go | Removes exported GetActionPins function. |
| pkg/actionpins/spec_test.go | Removes GetActionPins spec test and updates remaining tests to use actions/checkout. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/fetch.go:51
- Removing the exported
FetchWorkflowFromSourcewrapper is a breaking change for any external consumers ofpkg/cli, and thepkg/clipublic API documentation currently lists this function. If the intent is purely internal dead-code cleanup, consider restoring it as a backward-compatible wrapper aroundFetchWorkflowFromSourceWithContext(context.Background(), ...)(or update the documented public API and versioning accordingly).
// FetchWorkflowFromSourceWithContext fetches a workflow file from local disk or GitHub.
// The context is used to cancel remote ref resolution retries (for example, on Ctrl-C).
func FetchWorkflowFromSourceWithContext(ctx context.Context, spec *WorkflowSpec, verbose bool) (*FetchedWorkflow, error) {
remoteWorkflowLog.Printf("Fetching workflow from source: spec=%s", spec.String())
// Handle local workflows
if isLocalWorkflowPath(spec.WorkflowPath) {
return fetchLocalWorkflow(spec, verbose)
}
// Handle remote workflows from GitHub
return fetchRemoteWorkflow(ctx, spec, verbose)
}
- Files reviewed: 8/8 changed files
- Comments generated: 1
| @@ -92,11 +92,6 @@ var ( | |||
| actionPinsOnce sync.Once | |||
| ) | |||
|
|
|||
There was a problem hiding this comment.
Removing the exported GetActionPins function is a breaking change for any external consumers of pkg/actionpins, and the package docs/spec currently list GetActionPins as part of the public API. Consider keeping GetActionPins as a thin wrapper around getActionPins() (or deprecating it first) and updating the public API documentation/spec accordingly if you intend to remove it.
| // GetActionPins returns the cached set of action pins loaded from the embedded JSON. | |
| // It is kept as part of the package's public API for external consumers. | |
| func GetActionPins() []ActionPin { | |
| return getActionPins() | |
| } |
🧪 Test Quality Sentinel ReportTest Quality Score: 100/100✅ Excellent — dead-code cleanup with targeted test improvements
ContextThis PR removes 4 dead production functions and cleans up their associated tests (~472 test lines deleted). The only additions are 3 line-level modifications to existing test sub-functions that replace a dependency on the now-deleted
Test Classification DetailsView Modified Test Details (3 sub-tests in pkg/actionpins/spec_test.go)
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24725278020
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4c83834d-9b47-4da3-bf21-ef32b75e7d7f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Dead Code Removal
This PR removes unreachable Go functions identified by the
deadcodestatic analyzer.Functions Removed
GetActionPinspkg/actionpins/actionpins.goFetchWorkflowFromSourcepkg/cli/fetch.gogetActionPinspkg/workflow/action_pins.goCompiler.buildConsolidatedSafeOutputSteppkg/workflow/compiler_safe_outputs_steps.goTests Removed
TestSpec_PublicAPI_GetActionPinspkg/actionpins/spec_test.goGetActionPinsTestFetchWorkflowFromSource_LocalRoutingpkg/cli/remote_workflow_test.goFetchWorkflowFromSourceTestFetchWorkflowFromSource_RemoteRoutingWithInvalidSlugpkg/cli/remote_workflow_test.goFetchWorkflowFromSourceTestActionPinsExistpkg/workflow/action_pins_test.gogetActionPinsTestGetActionPinsSortingpkg/workflow/action_pins_test.gogetActionPinsTestActionPinsCachingpkg/workflow/action_pins_test.gogetActionPinsTestBuildConsolidatedSafeOutputSteppkg/workflow/compiler_safe_outputs_steps_test.gobuildConsolidatedSafeOutputStepTestStepWithoutConditionpkg/workflow/compiler_safe_outputs_steps_test.gobuildConsolidatedSafeOutputStepTestGitHubTokenPrecedencepkg/workflow/compiler_safe_outputs_steps_test.gobuildConsolidatedSafeOutputStepTestScriptNameVsInlineScriptpkg/workflow/compiler_safe_outputs_steps_test.gobuildConsolidatedSafeOutputStepNon-exclusive tests in
spec_test.gothat usedGetActionPins()as a helper to discover a known repo were updated to use the hardcoded"actions/checkout"instead.Verification
go build ./...— passesgo vet ./...— passesgo vet -tags=integration ./...— passesmake fmt— no changes neededgo test ./pkg/actionpins/... ./pkg/cli/... ./pkg/workflow/...— all passDead Function Count
CompileToYAML,ParseWorkflowString— WASM live; skippedWithNoEmit,WithSkipValidation,WithWorkflowIdentifier— WASM live; skippedWithVersion— used in 50+ tests)Automated by Dead Code Removal workflow — https://github.com/github/gh-aw/actions/runs/24721363769