[dead-code] chore: remove dead functions — 1 function removed#37331
Conversation
Remove GenerateHeredocDelimiterFromSeed (unreachable, test-only callers) and its 5 exclusive test functions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Removes an unused heredoc-delimiter helper (GenerateHeredocDelimiterFromSeed) and its associated unit tests from the workflow string utilities, simplifying the package and reducing maintenance surface.
Changes:
- Removed
GenerateHeredocDelimiterFromSeedfrompkg/workflow/strings.goand cleaned up now-unneeded imports (crypto/hmac,crypto/rand). - Removed the unit tests that exercised the deleted function from
pkg/workflow/strings_test.go.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/strings.go | Deletes the unused seeded-heredoc delimiter function and trims related imports. |
| pkg/workflow/strings_test.go | Removes tests that only covered the deleted function. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
🧪 Test Quality Sentinel completed test quality analysis. Test Quality Sentinel: PR #37331 is a deletion-only PR — 'GenerateHeredocDelimiterFromSeed' was removed from strings.go (36 lines) along with its 5 test functions from strings_test.go (65 lines). All changes are pure deletions with zero new or modified test functions added. No new behavioral contracts to score; test quality is unchanged. Analysis skipped. |
|
No ADR enforcement needed: PR #37331 does not have the 'implementation' label (has_implementation_label=false) and has 0 new lines of code in business logic directories (≤100 threshold, requires_adr_by_default_volume=false). |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out — one stale documentation entry found after scanning for all remaining references to the removed function.
📋 Key Themes & Highlights
Key Themes
- Stale documentation:
pkg/workflow/README.mdline 321 still listsGenerateHeredocDelimiterFromSeedin the function table, even though the function no longer exists after this PR.
Positive Highlights
- ✅ Zero remaining call sites in production code — the removal is safe
- ✅ All five covering tests removed alongside the function
- ✅ Unused imports (
crypto/hmac,crypto/rand) cleaned up correctly - ✅
go build ./...andgo vet ./...pass post-removal - ✅ The seeded approach is cleanly superseded by the content-addressed
GenerateHeredocDelimiterFromContent
Requested Change
Remove line 321 from pkg/workflow/README.md:
-| `GenerateHeredocDelimiterFromSeed` | `func(name, seed string) string` | Generates a stable heredoc delimiter |Otherwise the public API table documents a function that does not exist, which will confuse callers.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 387.4K · 116.2 AIC · ⌖ 12.9 AIC
There was a problem hiding this comment.
Stale API documentation will mislead consumers of this package. pkg/workflow/README.md still lists GenerateHeredocDelimiterFromSeed as an exported function (line 321), and its active replacement GenerateHeredocDelimiterFromContent is absent from the same table.
💡 Required fix
In pkg/workflow/README.md, replace line 321:
| `GenerateHeredocDelimiterFromSeed` | `func(name, seed string) string` | Generates a stable heredoc delimiter |
with:
| `GenerateHeredocDelimiterFromContent` | `func(name, content string) string` | Generates a stable heredoc delimiter derived from the wrapped content |
GenerateHeredocDelimiterFromContent is already actively used in mcp_setup_generator.go and several other callers — it just was never documented when it replaced the seed-based variant. A reader following the current README would attempt to call a function that no longer compiles.
The code deletions themselves are clean: the function has no remaining Go callers, the crypto/hmac/crypto/rand imports are correctly removed, and all five test functions are properly trimmed.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 155.4K · ⌖ 12.8 AIC
chore: remove dead functions
Summary
Removes the unused public function
GenerateHeredocDelimiterFromSeedfrompkg/workflow/strings.gotogether with its two no-longer-needed imports (crypto/hmac,crypto/rand), and deletes the five accompanying test functions that covered its full behaviour surface.What changed and why
pkg/workflow/strings.goGenerateHeredocDelimiterFromSeed(HMAC-SHA256 seeded heredoc-delimiter generator) and removed thecrypto/hmac/crypto/randimports that were only required by that function.pkg/workflow/strings_test.goBreaking changes
Callers must remove all references to
GenerateHeredocDelimiterFromSeedbefore upgrading to this version.Checklist
crypto/hmac,crypto/rand) cleaned up — no orphan imports remainGenerateHeredocDelimiterFromSeed(search dependents / downstream repos)Commit