-
Notifications
You must be signed in to change notification settings - Fork 298
Closed
Description
🔍 Duplicate Code Detected: Schedule cron normalization duplicated in preprocessScheduleFields
Analysis of commit c651689
Assignee: @copilot
Summary
The schedule string parsing/warning/scatter logic in preprocessScheduleFields is duplicated, increasing the chance of inconsistent fixes between on shorthand and on.schedule string handling.
Duplication Details
Pattern: Repeated schedule string parsing and cron normalization
- Severity: Medium
- Occurrences: 2
- Locations:
pkg/workflow/schedule_preprocessing.go:62-138pkg/workflow/schedule_preprocessing.go:155-223
- Code Sample:
parsedCron, original, err := parser.ParseSchedule(onStr)
if parser.IsDailyCron(parsedCron) && !parser.IsFuzzyCron(parsedCron) { c.addDailyCronWarning(parsedCron) }
// ... hourly/weekly warnings + scatter fuzzy cron
if parser.IsFuzzyCron(parsedCron) { return fmt.Errorf("fuzzy cron expression '%s' must be scattered...", parsedCron) }
if !parser.IsCronExpression(parsedCron) { return fmt.Errorf("invalid cron expression '%s'", parsedCron) }
scheduleArray := []any{{"cron": parsedCron}}Impact Analysis
- Maintainability: Warning/validation updates must be duplicated; easy for the branches to drift.
- Bug Risk: Fixes applied to one branch may be missed in the other, causing different behavior depending on whether
onis a string or nested schedule string. - Code Bloat: 60+ nearly identical lines enlarge
preprocessScheduleFields, reducing readability.
Refactoring Recommendations
- Extract a helper (e.g.,
normalizeScheduleString) that handles parsing, warning emission, fuzzy scattering, and friendly format capture. Reuse it for both top-levelonstrings andschedulestring values.- Estimated effort: 1–2 hours
- Benefits: single source of truth for cron validation; easier to extend warning logic.
- Optionally centralize the daily/hourly/weekly warning emission into a small helper to keep
preprocessScheduleFieldsfocused on dispatching to handlers.
Implementation Checklist
- Review duplication findings
- Prioritize refactoring tasks
- Create refactoring plan
- Implement changes
- Update tests
- Verify no functionality broken
Analysis Metadata
- Analyzed Files: 2 (.go)
- Detection Method: Manual semantic review of schedule preprocessing logic
- Commit: c651689
- Analysis Date: 2025-12-21T19:27:46Z
AI generated by Duplicate Code Detector
Reactions are currently unavailable