[dead-code] chore: remove dead functions — 1 function removed#42778
Conversation
Remove unreachable isRepositorySkillSpec and its exclusive test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #42778 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. |
There was a problem hiding this comment.
Pull request overview
Removes a dead helper used for classifying skill spec strings and deletes the now-obsolete unit test, keeping the skills frontmatter codepath leaner.
Changes:
- Removed
isRepositorySkillSpecfrompkg/workflow/skills_frontmatter.go. - Removed
TestIsRepositorySkillSpecfrompkg/workflow/skills_frontmatter_test.go.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/skills_frontmatter.go | Drops an unused skill-spec classification helper. |
| pkg/workflow/skills_frontmatter_test.go | Removes the unit test that exclusively covered the deleted helper. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Low
| return strings.Count(base, "/") == 1 | ||
| } | ||
|
|
||
| func parseRawSkillReferences(rawSkills []any) []SkillReference { |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Dead code removal looks correct
The removed function and its dedicated test are cleanly excised. Build and vet checks pass. No callers remain. Nothing blocking — the ADR staleness concern is already noted in an existing inline comment.
🔎 Code quality review by PR Code Quality Reviewer · 20.6 AIC · ⌖ 7.09 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Clean dead-code removal. isRepositorySkillSpec has no remaining callers in Go source (ADR reference is documentation-only), and the co-located test is correctly removed alongside it. Build, vet, and fmt checks all pass per PR description. Nothing to block.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 11 AIC · ⌖ 6.56 AIC · ⊞ 4.9K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — this is a clean, well-scoped dead code removal. No blocking issues found.
📋 Summary
What was removed
isRepositorySkillSpec— had no call sites remaining in the codebaseTestIsRepositorySkillSpec— exclusively tested the removed function, so its deletion is correct and expected
Positive Highlights
- ✅ Automated dead-code detection paired with manual verification (
go build,go vet,make fmt) - ✅ Test removal is cleanly paired with the function removal — no orphaned tests
- ✅ Scope is tightly bounded to two files with zero additions
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 29.2 AIC · ⌖ 6.98 AIC · ⊞ 6.6K
Comment /matt to run again
🧪 Test Quality Sentinel Report➖ No new tests to score — dead-code removal only.
📊 Metrics (0 new tests)
No new test functions were added or changed. The 5 deleted lines in Verdict
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
🎉 This pull request is included in a new release. Release: |
Functions Removed
isRepositorySkillSpecpkg/workflow/skills_frontmatter.goTests Removed
TestIsRepositorySkillSpec(exclusively tested the removed function)Verification
go build ./...go vet ./...go vet -tags=integration ./...make fmthttps://github.com/github/gh-aw/actions/runs/28527555147