Uh oh!
There was an error while loading. Please reload this page.
[dead-code] chore: remove dead functions — 2 functions removed - #54023
Conversation
Remove 2 unreachable functions identified by deadcode analyzer: - FormatLogEntry (pkg/cli/log_entry.go) - IsKnownGitHubWebhookEvent (pkg/workflow/github_app_requirements.go) Also removes their exclusive test-only callers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
✅ 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).
|
✅ PR Code Quality Reviewer completed the code quality review.
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
There was a problem hiding this comment.
Clean removal of two unused exported functions (FormatLogEntry and IsKnownGitHubWebhookEvent) along with their tests and the now-unused fmt import. No issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 15.1 AIC · ⌖ 8.75 AIC · ⊞ 5.7K
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.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — this is a clean dead-code removal. No issues found; approving.
📋 Review Summary
What was removed
FormatLogEntry(pkg/cli/log_entry.go) — exported wrapper with no callers; its testTestFormatLogEntryIsGenericAcrossSourcesexercised the wrapper's formatting behaviour (which is fully covered by the underlyingLogEntryinterface accessor tests that remain).IsKnownGitHubWebhookEvent(pkg/workflow/github_app_requirements.go) — thin exported shim over the unexportedisKnownGitHubEvent; removing the public surface is the right call since no callers existed.
Positive Highlights
- ✅ Both removals are verified dead (no references anywhere in the codebase)
- ✅ Unused
"fmt"import correctly cleaned up alongside the function removal - ✅ PR description documents skipped candidates with clear rationale — good discipline
- ✅ Verification checklist (build, vet, fmt, tests) is thorough and honest about pre-existing failures
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 17 AIC · ⌖ 9.85 AIC · ⊞ 7.8K
Comment /matt to run again
There was a problem hiding this comment.
No blocking issues in the changed lines. This PR only removes two apparently unused wrappers and their direct tests, and I did not find a correctness, performance, or maintainability regression introduced by the deletion set.
Review notes
FormatLogEntryremoval is consistent with the remaining code inpkg/cli/log_entry.go; the interface and per-type accessors still compile as the only generic surface left in this diff.IsKnownGitHubWebhookEventremoval also looks safe within the changed lines becauseNormalizeGitHubAppWebhookEventsalready calls the unexported helper directly and there are no in-repo call sites to the exported wrapper in this branch.- I did not see a changed-line gap that merits a blocking comment.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 4.2 AIC · ⌖ 6.86 AIC · ⊞ 7K
Comment /review to run again
There was a problem hiding this comment.
Pull request overview
Removes two unused helper functions and their dedicated tests.
Changes:
- Removes
FormatLogEntryand its test. - Removes
IsKnownGitHubWebhookEventand its test.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/log_entry.go | Removes generic log formatter. |
pkg/cli/log_entry_test.go | Removes formatter coverage. |
pkg/workflow/github_app_requirements.go | Removes webhook-event wrapper. |
pkg/workflow/github_app_requirements_test.go | Removes wrapper coverage. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
| @@ -1,7 +1,6 @@ | |||
| package cli | |||
| import ( | |||
🎉 This pull request is included in a new release. Release: |
test> Generated by PR Description Updater for #54023 · auto · 32 AIC · ⌖ 4.22 AIC · ⊞ 7.6K · ◷