-
Notifications
You must be signed in to change notification settings - Fork 341
Duplicate Code: Safe-Output Config Parsing Repeated Across create-issue/create-discussion/create-pull-request #23228
Copy link
Copy link
Open
Labels
enhancementNew feature or requestNew feature or request
Description
Overview
The parse*Config implementations for safe-output creation actions repeat the same parse pipeline across three files with mostly field-level differences. This creates a maintenance hotspot where validation and parsing behavior can drift.
The duplicated sequence appears in:
create-issuecreate-discussioncreate-pull-request
Critical Information
- Severity: Medium
- Occurrences: 3
- Duplication size: ~20-30 lines repeated per implementation (core parsing flow)
- Risk: Inconsistent behavior when updating preprocessing/unmarshal/default logic
Duplication Details
Pattern: repeated parse pipeline in parse*Config methods:
- Check key exists in
outputMap - Extract
configData preprocessExpiresField(...)preprocessBoolFieldAsString(...)looppreprocessIntFieldAsString(..., "max", ...)unmarshalConfig(...)with identical fallback handling- Default
maxhandling
Locations:
pkg/workflow/create_issue.go:29pkg/workflow/create_discussion.go:31pkg/workflow/create_pull_request.go:42
Example duplicated block
// Unmarshal into typed config struct
var config CreateIssuesConfig
if err := unmarshalConfig(outputMap, "create-issue", &config, createIssueLog); err != nil {
createIssueLog.Printf("Failed to unmarshal config: %v", err)
// For backward compatibility, handle nil/empty config
config = CreateIssuesConfig{}
}
// Set default max if not specified
if config.Max == nil {
config.Max = defaultIntStr(1)
}Equivalent logic exists in:
CreateDiscussionsConfigunmarshal/default pathCreatePullRequestsConfigunmarshal/default path
Impact Analysis
- Maintainability: Any parser behavior change must be applied in 3 places.
- Bug risk: Easy to introduce subtle divergence in bool/int preprocessing or defaulting semantics.
- Code bloat: Repeated orchestration code obscures entity-specific rules.
Refactoring Recommendations
- Extract shared parse orchestration helper
- Suggested location:
pkg/workflow/safe_outputs_parser_shared.go - Candidate API: generic/helper that accepts key name, logger, bool fields, and a post-process callback for entity-specific defaults.
- Estimated effort: Medium (3-5 hours)
- Keep entity-specific behavior in focused hooks
- Example hooks:
- assignee normalization (
create-issue) - category normalization/fallback (
create-discussion) - reviewers/protected-files logic (
create-pull-request)
- assignee normalization (
- Benefit: shared validation path + explicit per-entity extension points
Implementation Checklist
- Define common parse pipeline helper
- Migrate
parseIssuesConfig - Migrate
parseDiscussionsConfig - Migrate
parsePullRequestsConfig - Add regression tests for shared preprocessing/default behavior
- Verify no behavior drift in existing safe-output tests
Analysis Metadata
- Analyzed Files: 6 focused files (
pkg/workflow/*.goparser files + related checks) - Detection Method: Serena semantic symbol analysis + pattern search
- Commit:
e4a7c883cf12265be930e4c387d2ff06edd533cf - Analysis Date: 2026-03-27 UTC
References:
Generated by Duplicate Code Detector · ◷
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or request
Type
Fields
Give feedbackNo fields configured for issues without a type.