[dead-code] chore: remove dead functions — 4 functions removed - #49801
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. No new tests were added or modified in this PR. The PR removes dead code (4 functions) and their associated tests (2 test functions). Test Quality Sentinel analysis skipped. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
There was a problem hiding this comment.
Pull request overview
Removes four apparently unused workflow helpers, but they are documented public APIs and require compatibility handling.
Changes:
- Removes three permission factories.
- Removes deprecated
RunGHWithHost. - Removes two related specification tests.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/spec_test.go |
Removes permission factory tests. |
pkg/workflow/permissions_factory.go |
Removes three exported factories. |
pkg/workflow/github_cli.go |
Removes an exported GitHub CLI helper. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suppressed comments (2)
pkg/workflow/permissions_factory.go:170
- This exported factory remains listed in the package's Public API (
pkg/workflow/README.md:195), and external consumers can importpkg/workflow; an in-repo usage scan cannot establish that it is dead. The repository's policy (docs/adr/26551-migrate-github-app-token-input-to-client-id.md:90-93) requires a compatibility alias or an explicit breaking release, so retain this function and its test here.
// NewPermissionsIssuesWrite creates permissions with issues: write only.
pkg/workflow/permissions_factory.go:125
- This symbol is part of the documented public API (
pkg/workflow/README.md:192), so having no in-repo caller does not make it safe to remove: external imports will stop compiling. Per the repository's breaking-API policy indocs/adr/26551-migrate-github-app-token-input-to-client-id.md:90-93, retain a compatibility function (and its spec test) unless removal is deferred to an explicit breaking release.
// NewPermissionsContentsReadPRWrite creates permissions with contents: read and pull-requests: write
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
| }) | ||
| } | ||
|
|
||
| // NewPermissionsActionsWrite creates permissions with actions: write |
| return output, enrichGHError(err) | ||
| } | ||
|
|
||
| // RunGHContextWithHost executes a gh CLI command with context support, a spinner, |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean dead-code removal. All 4 removed functions were either explicitly deprecated or unused, tests are properly removed alongside their implementations, and no call sites are affected. LGTM.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 11.4 AIC · ⌖ 10.1 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — clean, well-scoped dead-code removal. No actionable issues.
📋 Highlights
- ✅ Each removed function had its corresponding test removed — no orphaned tests
- ✅
RunGHWithHostwas already markedDeprecatedpointing toRunGHContextWithHost - ✅ Build and vet verified in CI before opening PR
- ✅ Codebase is simpler and easier to navigate after this change
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 18 AIC · ⌖ 18.2 AIC · ⊞ 7.1K
Comment /matt to run again
There was a problem hiding this comment.
Review: clean dead-code removal
Verified all four removed functions/tests have no remaining references anywhere in the codebase (grep for RunGHWithHost, NewPermissionsContentsReadIssuesWritePRWrite, NewPermissionsContentsReadIssuesWriteDiscussionsWrite, NewPermissionsContentsReadProjectsWrite outside the diff returns nothing). go build ./pkg/workflow/... succeeds. This is a purely subtractive change (0 additions, 89 deletions) with no logic modifications — nothing to flag.
Details
- No callers of the removed exported functions exist in production or test code.
- Corresponding spec tests were removed alongside their functions, keeping test/impl parity.
- Build and vet pass per PR checklist.
🔎 Code quality review by PR Code Quality Reviewer · auto · 22.3 AIC · ⌖ 4.25 AIC · ⊞ 7.9K
Comment /review to run again
|
🎉 This pull request is included in a new release. Release: |
Functions Removed
NewPermissionsContentsReadIssuesWritePRWritepkg/workflow/permissions_factory.goNewPermissionsContentsReadIssuesWriteDiscussionsWritepkg/workflow/permissions_factory.goNewPermissionsContentsReadProjectsWritepkg/workflow/permissions_factory.goRunGHWithHostpkg/workflow/github_cli.goTests Removed
TestSpec_Permissions_ContentsReadIssuesWriteDiscussionsWrite(pkg/workflow/spec_test.go)TestSpec_Permissions_ContentsReadProjectsWrite(pkg/workflow/spec_test.go)Verification
go build ./...go vet ./...go vet -tags=integration ./...make fmthttps://github.com/github/gh-aw/actions/runs/30752567710