[dead-code] chore: remove dead functions — 5 functions removed#47339
Conversation
Remove 5 unreachable Print* functions from pkg/console/print.go that had no callers outside of test files, along with their exclusive tests. Functions removed: - PrintError - PrintTableHeaderStderr - PrintErrorTextStderr - PrintProgressMessage, PrintPromptMessage, PrintVerboseMessage Tests removed: TestPrintErrorNewline, TestPrintTableHeaderStderr, TestPrintErrorTextStderr, TestPrintProgressMessage, TestPrintPromptMessage, TestPrintVerboseMessage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
closed:unknown
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #47339 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). The 2 changed files are outside of monitored business logic directories (src/, lib/, pkg/, internal/, app/, core/, domain/, services/, api/). |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. This PR removes dead code and corresponding dead tests (53 lines deleted from print_test.go). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Removes unused console printing helpers and their tests.
Changes:
- Removes six
Print*helpers. - Removes their associated tests.
- Leaves a broken README example and removes
PrintVerboseMessagecontrary to the stated scope.
Show a summary per file
| File | Description |
|---|---|
pkg/console/print.go |
Removes six stderr printing helpers. |
pkg/console/print_test.go |
Removes corresponding helper tests. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/console/print.go:38
- This hunk also removes
PrintVerboseMessage, making the change six function removals rather than the five stated in the title and description; the description explicitly says this function is retained. Restore it and its test, or update the PR scope and rationale if the sixth removal is intentional.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Medium
| func PrintCommandMessage(command string) { | ||
| fmt.Fprintln(stderr, FormatCommandMessageStderr(command)) | ||
| } | ||
|
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — clean dead-code removal with one documentation gap to address.
📋 Key Themes & Highlights
Issues
- Stale README example:
pkg/console/README.mdline 408 still referencesPrintProgressMessagewhich was removed. This will mislead future package consumers. (Already flagged in an existing review comment — needs follow-up.) - PR description inaccuracy: The description states
PrintVerboseMessagewas retained, but the diff shows it was also removed (testTestPrintVerboseMessagedeleted, function gone fromprint.go). The note should be corrected.
Positive Highlights
- ✅ All removed functions were genuinely dead — no production callers found
- ✅ Tests removed alongside their functions (no orphaned tests)
- ✅ Build, vet, fmt, and package tests verified before submitting
- ✅ Surgical, minimal diff with no collateral changes
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 25.3 AIC · ⌖ 4.54 AIC · ⊞ 6.7K
Comment /matt to run again
There was a problem hiding this comment.
LGTM — clean dead-code removal.
All 6 deleted Print* functions are confirmed to have zero production callers (verified via grep). Build and tests passed per CI run linked in the PR. The corresponding Format* peers are either still referenced elsewhere or can be cleaned up in a follow-up.
Minor note: the PR description says
PrintVerboseMessagewas "retained" but the diff removes it and its test — the description is slightly inaccurate, though the code change itself is correct.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 43.3 AIC · ⌖ 4.49 AIC · ⊞ 5.7K
Comment /review to run again
There was a problem hiding this comment.
Review
The removals are correct — no callers exist for any of the 5 deleted functions. However, pkg/console/README.md (line 408) still contains a usage example calling console.PrintProgressMessage("Downloading release..."), which no longer exists. This stale documentation reference must be updated before merging.
Also: the PR description states PrintVerboseMessage was retained, but it was actually removed along with its test. The removal is correct (no callers), but the description is inaccurate.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 36.8 AIC · ⌖ 4.48 AIC · ⊞ 5K
|
🎉 This pull request is included in a new release. Release: |
Summary
Removes dead
Print*wrapper functions frompkg/console/print.gothat were no longer called anywhere in the codebase. Each removed function was a thinfmt.Fprint/fmt.Fprintlnwrapper around an existingFormat*function.Changes
pkg/console/print.go— 6 functions deleted:PrintError(err CompilerError)PrintTableHeaderStderr(text string)PrintErrorTextStderr(text string)PrintProgressMessage(message string)PrintPromptMessage(message string)PrintVerboseMessage(message string)pkg/console/print_test.go— corresponding unit tests for all removed functions deleted.Impact
pkg/console.Format*functions remain available for callers that need the formatted string without printing.