-
Notifications
You must be signed in to change notification settings - Fork 167
Closed as not planned
Closed as not planned
Copy link
Labels
Description
🔍 Duplicate Code Detected: Expires Pre-Processing Block
Analysis of commit 0c8e423
Assignee: @copilot
Summary
The same pre-processing block for parsing and normalizing the expires field is duplicated in multiple safe-output config parsers. This logic is >10 lines and is effectively identical in both files, creating a maintenance hotspot when expiration semantics change.
Duplication Details
Pattern: expires pre-processing before unmarshaling
- Severity: Medium
- Occurrences: 2
- Locations:
pkg/workflow/create_issue.go:37(approx. 37-55)pkg/workflow/create_discussion.go:40(approx. 40-58)
- Code Sample:
// Pre-process the expires field (convert to hours before unmarshaling)
expiresDisabled := false
if configData != nil {
if expires, exists := configData["expires"]; exists {
// Always parse the expires value through parseExpiresFromConfig
// This handles: integers (days), strings (time specs like "48h"), and boolean false
expiresInt := parseExpiresFromConfig(configData)
if expiresInt == -1 {
// Explicitly disabled with false
expiresDisabled = true
configData["expires"] = 0
} else if expiresInt > 0 {
configData["expires"] = expiresInt
} else {
// Invalid or missing - set to 0
configData["expires"] = 0
}
log.Printf("Parsed expires value %v to %d hours (disabled=%t)", expires, expiresInt, expiresDisabled)
}
}Impact Analysis
- Maintainability: Any change to expiration parsing rules requires editing multiple files and keeping logic in sync.
- Bug Risk: Divergence is likely over time (e.g., adding new time spec formats in one parser but not another).
- Code Bloat: ~18 lines duplicated across two files; more likely to expand with future features.
Refactoring Recommendations
-
Extract shared helper
- Extract a shared helper in
pkg/workflow/config_helpers.goorpkg/workflow/time_delta.gothat returns(expiresHours int, expiresDisabled bool)and optionally updatesconfigData["expires"]. - Estimated effort: 1-2 hours.
- Benefits: Single source of truth for expires parsing, easier future changes.
- Extract a shared helper in
-
Centralize logging and mutation
- Wrap the log message and config mutation inside the helper to keep logging consistent and avoid repeated
configDatachecks. - Estimated effort: <1 hour.
- Benefits: Clearer parsing flow in config parsing functions.
- Wrap the log message and config mutation inside the helper to keep logging consistent and avoid repeated
Implementation Checklist
- Review duplication findings
- Prioritize refactoring tasks
- Create refactoring plan
- Implement changes
- Update tests
- Verify no functionality broken
Analysis Metadata
- Analyzed Files: 3 (create_issue.go, create_discussion.go, create_pull_request.go)
- Detection Method: Manual code review (Serena LSP unavailable)
- Commit: 0c8e423
- Analysis Date: 2026-02-04
AI generated by Duplicate Code Detector
Reactions are currently unavailable