[dead-code] chore: remove dead functions — 5 functions removed - #58822
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict
I did not find a blocking issue in this dead-code removal.
The deletions are still covered by remaining call sites and tests
I checked the removed wrappers and helper predicates against the current callers in pkg/cli/, and every remaining path now uses the lower-level functions directly (confirmAddRepositoryInitialization + applyAddRepositoryInitialization, inspectAddWorkingTreeAtRoot, manifestIncludeWildcardParent, extractResourceEntries, and applyGradersArtifact / isEvalsArtifactRequested). The deleted tests were also targeting wrappers or aliases whose behavior is still exercised indirectly by the surviving tests around the active entry points, so this PR does not obviously reduce correctness coverage on the changed code paths.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 29.3 AIC · ⌖ 7.35 AIC · ⊞ 21.8K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Reviewed this dead-code removal PR against /codebase-design and /diagnosing-bugs — no actionable issues found.
📋 Verification
Confirmed via repo-wide search that none of the five removed functions (confirmAndInitializeAddRepository, inspectAddWorkingTree, isManifestIncludeWildcard, isGradersArtifactRequested, extractResources) have any remaining callers in non-test code. Their corresponding tests are removed alongside, keeping test/code coverage consistent. go build ./... succeeds.
This is a purely subtractive, mechanical change with no new abstractions, no behavior change, and no logic to test — so /tdd and /codebase-design guidance don't surface anything to improve here.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 20.6 AIC · ⌖ 14.5 AIC · ⊞ 10.3K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
Several deleted tests uniquely cover production functions that remain active.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes five unused CLI helper functions and their associated tests.
Changes:
- Deletes dead wrapper/helper functions.
- Removes related tests, including some still covering active implementations.
File summaries
| File | Description |
|---|---|
pkg/cli/resources.go |
Removes extractResources. |
pkg/cli/remote_workflow_test.go |
Removes resource extraction tests. |
pkg/cli/logs_artifact_set.go |
Removes unused graders-request helper. |
pkg/cli/logs_artifact_set_test.go |
Removes its unit test. |
pkg/cli/add_package_manifest_includes.go |
Removes wildcard wrapper. |
pkg/cli/add_package_manifest_wildcard_test.go |
Removes wildcard validation cases. |
pkg/cli/add_interactive_git.go |
Removes working-tree wrapper. |
pkg/cli/add_interactive_git_test.go |
Removes working-tree inspection tests. |
pkg/cli/add_init.go |
Removes initialization wrapper. |
pkg/cli/add_command_test.go |
Removes initialization behavior tests. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| }) | ||
| } | ||
|
|
||
| func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *testing.T) { |
| } | ||
| } | ||
|
|
||
| func TestBuildWorkingTreeResolutionOptions(t *testing.T) { |
| } | ||
| } | ||
|
|
||
| func TestExtractManifestIncludesWithWildcard(t *testing.T) { |
| assert.Equal(t, []string{"./graders/example-operational-value.sh"}, resources) | ||
| } | ||
|
|
||
| func TestFetchAndSaveRemoteResources_InstallsAndRestoresGraderEvaluator(t *testing.T) { |
|
🎉 This pull request is included in a new release. Release: |
Functions Removed
confirmAndInitializeAddRepositorypkg/cli/add_init.goinspectAddWorkingTreepkg/cli/add_interactive_git.goisManifestIncludeWildcardpkg/cli/add_package_manifest_includes.goisGradersArtifactRequestedpkg/cli/logs_artifact_set.goextractResourcespkg/cli/resources.goTests Removed
TestConfirmAndInitializeAddRepository(pkg/cli/add_command_test.go)TestInspectAddWorkingTree(pkg/cli/add_interactive_git_test.go)TestIsManifestIncludeWildcard(pkg/cli/add_package_manifest_wildcard_test.go)TestIsGradersArtifactRequested(pkg/cli/logs_artifact_set_test.go)TestExtractResources_BasicList,TestExtractResources_IncludesGraderEvaluator,TestExtractResources_IncludesDisabledGraderEvaluator,TestExtractResources_MacroRejected,TestExtractResources_AllMacrosRejected,TestExtractResources_NoResourcesField(pkg/cli/remote_workflow_test.go)Verification
go build ./...go vet ./...go vet -tags=integration ./...make fmthttps://github.com/github/gh-aw/actions/runs/33971358925