Skip to content

[dead-code] chore: remove dead functions — 4 functions removed - #49357

Merged
pelikhan merged 2 commits into
mainfrom
chore/dead-code-removal-1785510590-95ac5acdb6b3a671
Aug 1, 2026
Merged

[dead-code] chore: remove dead functions — 4 functions removed#49357
pelikhan merged 2 commits into
mainfrom
chore/dead-code-removal-1785510590-95ac5acdb6b3a671

Conversation

@github-actions

@github-actions github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Generated by PR Description Updater for #49357 · auto · 51.9 AIC · ⊞ 6.8K ·

Remove 4 unreachable functions identified by deadcode analyzer:
- checkForUpdates (pkg/cli/update_check.go)
- formatTableHeaderWithTTY (pkg/console/console.go)
- formatErrorTextWithTTY (pkg/console/console.go)
- NewPermissionsIssuesWriteDiscussionsWritePRWrite (pkg/workflow/permissions_factory.go)

Also removed exclusive tests for the deleted functions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review July 31, 2026 20:56
Copilot AI review requested due to automatic review settings July 31, 2026 20:56
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Test Quality Sentinel completed test quality analysis.

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

PR Code Quality Reviewer completed the code quality review.

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

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).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes four unused helper functions and their obsolete unit tests.

Changes:

  • Removes unused update-check and console-formatting wrappers.
  • Removes an unused permissions factory and associated tests.
Show a summary per file
File Description
pkg/cli/update_check.go Removes the unused synchronous update-check wrapper.
pkg/cli/update_check_test.go Removes tests specific to that wrapper.
pkg/console/console.go Removes two unused TTY formatting helpers.
pkg/console/console_formatting_test.go Removes tests for those helpers.
pkg/workflow/permissions_factory.go Removes an unused permissions factory.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor Author

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🧪 Test Quality Sentinel Report

Test Quality Score: N/A — Deletions Only

Dead-code removal PR. No new or modified test functions were added. Test deletions only (139 lines removed from test files).

📊 Summary
Metric Value
Files analyzed 2 test files
New tests added 0
Modified tests 0
Test deletions 139 lines
🚨 Violations 0

Modified test files:

  • pkg/cli/update_check_test.go (deletions)
  • pkg/console/console_formatting_test.go (deletions)

Verdict

passed. No new or modified behavioral tests to evaluate. Test deletions reflect removal of dead code being eliminated in this PR.

🧪 Test quality analysis by Test Quality Sentinel · haiku45 · 7.25 AIC · ⊞ 8.4K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: Dead-code removal PR. No new tests added or modified. No violations detected.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Clean dead-code removal — all deleted functions were unused and their tests covered only those removed functions. Build and vet pass per PR description. LGTM.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 13.5 AIC · ⊞ 5.3K

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

✅ Verified clean — approving

All 4 removed functions (checkForUpdates, formatTableHeaderWithTTY, formatErrorTextWithTTY, NewPermissionsIssuesWriteDiscussionsWritePRWrite) confirmed dead: no remaining references anywhere in the repo (grep across .go/.md/.yml), no reflection/string-keyed dispatch, no partial removal (their shared helpers like applyStyleWithTTY and checkForUpdatesWithContext remain in active use elsewhere). Independently re-ran go build ./... and go vet ./... — both pass clean.

Details
  • checkForUpdates was a thin wrapper delegating to checkForUpdatesWithContext, which is still called via a function-variable indirection elsewhere and covered by its own tests.
  • The two console TTY formatting helpers were only used by their own now-removed tests; applyStyleWithTTY (the underlying primitive) is still used by other call sites.
  • NewPermissionsIssuesWriteDiscussionsWritePRWrite was unreferenced; all NewPermissions* factories are invoked by direct Go identifier, not by name/reflection, so removal cannot silently break dynamic dispatch.
  • No additions in this diff, only deletions — nothing to review for new logic bugs.

🔎 Code quality review by PR Code Quality Reviewer · auto · 21.6 AIC · ⊞ 7.8K
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Skills-Based Review 🧠

Applied /codebase-design — clean dead-code removal, no issues found.

📋 Summary
  • ✅ All 4 removed functions were unexported wrappers with no remaining callers
  • ✅ Corresponding tests removed in lockstep — no orphaned test coverage
  • ✅ Build, vet, and fmt verified by author

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 15.5 AIC · ⊞ 7K
Comment /matt to run again

@github-actions

Copy link
Copy Markdown
Contributor Author

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Hey 👋 — thanks for the dead code cleanup! This PR looks great and is ready for review. Here's what stands out:

Well-focused — removes 4 dead functions and their corresponding tests cleanly
Comprehensive description — lists functions removed, tests updated, and verification of all checks passing
Proper test maintenance — test functions for dead code were removed alongside the code itself
Verification includedgo build, go vet, and make fmt all pass

This aligns perfectly with the project's agentic development workflow. The PR is in great shape and ready for maintainer review.

Generated by ✅ Contribution Check · auto · 54.6 AIC · ⊞ 8.7K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

Warning

threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.

Details

The threat detection engine failed to produce results.

Review the workflow run logs for details.

@copilot please run the pr-finisher skill on this PR, investigate the failing check below, refresh the branch if GitHub allows it, and rerun the relevant checks once ready.

Failed checks:

Run details: https://github.com/github/gh-aw/actions/runs/30669315501

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.62 AIC · ⊞ 8.1K ·
Comment /souschef to run again

Copilot AI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

[!WARNING]
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding....

PR Finisher summary

  • Reviews — All review threads are resolved and approving. No unresolved threads.
  • Checks (local)make fmt, make lint, and make test-unit (full suite) all pass locally. No failures in the code changes.
  • Checks (CI) — Stale after agent push; needs human re-trigger. Prior failure root-caused: the build check at run 30641904040/job/91269150420 failed due to a transient cache restore timeout (ETIMEDOUT on GetCacheEntryDownloadURL), not a code issue. Not reproducible locally.
  • Mergeable — PR is OPEN, not draft, and branch is already up to date with main (merge commit ec7d024 already present).

Actions taken: No code changes were needed. Local validation (fmt / lint / test-unit) confirmed the PR is clean.

Hand-off: CI must be re-triggered by a maintainer (close/reopen PR or workflow_dispatch) before merge — the prior build failure was an infra flake (cache network timeout) unrelated to these changes.

@pelikhan

pelikhan commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@copilot doesn't compile do a background checkForUpdates ?

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

@copilot doesn't compile do a background checkForUpdates ?

Yes — compile does a background update check, but through a dedicated function (StartCompileUpdateCheck in compile_update_check.go), not through the removed checkForUpdates. The removed function was a thin synchronous wrapper around checkForUpdatesWithContext that was never called from anywhere in production code. The actual background paths are:

  • compileStartCompileUpdateCheck (cmd/gh-aw/main.go:347) — its own dedicated background update check
  • validateCheckForUpdatesAsync (validate_command.go:48) — the async version in update_check.go

The removed checkForUpdates was confirmed dead by the static analysis tool and is not in either of those call chains.

Copilot AI requested a review from pelikhan August 1, 2026 00:47
@pelikhan
pelikhan merged commit 465bf7d into main Aug 1, 2026
2 checks passed
@pelikhan
pelikhan deleted the chore/dead-code-removal-1785510590-95ac5acdb6b3a671 branch August 1, 2026 00:51
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

🎉 This pull request is included in a new release.

Release: v0.84.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants