Skip to content

[duplicate-code] Duplicate Code Detected: Safe-output list job builders #6296

Description

@github-actions

🔍 Duplicate Code Detected: Safe-output list job builders

*Analysis of commit *

Assignee: @copilot

Summary

The builder and parser logic for list-based safe-output jobs (add-labels, assign-milestone, assign-to-user) are nearly identical, with repeated max/default handling, env var construction via BuildListJobEnvVars, and buildSafeOutputJob wiring. This duplication spreads the same logic across three files, making maintenance and behavior changes error-prone.

Duplication Details

Pattern: Repeated list-job parsing and job construction

  • Severity: Medium
  • Occurrences: 3 builders + 3 parsers with largely identical structure
  • Locations:
    • pkg/workflow/add_labels.go:18-42 and pkg/workflow/add_labels.go:45-92
    • pkg/workflow/assign_milestone.go:14-35 and pkg/workflow/assign_milestone.go:38-80
    • pkg/workflow/assign_to_user.go:18-70 and pkg/workflow/assign_to_user.go:73-98
  • Code Sample:
listJobConfig := ListJobConfig{
    SafeOutputTargetConfig: cfg.SafeOutputTargetConfig,
    Allowed:                cfg.Allowed,
}
customEnvVars := BuildListJobEnvVars("GH_AW_LABELS", listJobConfig, maxCount)
customEnvVars = append(customEnvVars, c.buildStandardSafeOutputEnvVars(data, cfg.TargetRepoSlug)...)
outputs := map[string]string{
    "labels_added": "${{ steps.add_labels.outputs.labels_added }}",
}
return c.buildSafeOutputJob(data, SafeOutputJobConfig{ /* job name/id/script/permissions */ })

Impact Analysis

  • Maintainability: Fixes to list-job behavior (defaults, env wiring, permissions) must be repeated three times, increasing drift risk.
  • Bug Risk: Inconsistent defaults (e.g., max counts, target handling) can emerge if one copy is updated without the others.
  • Code Bloat: Duplicated ~40+ lines across three files; changes inflate review surface and reduce clarity of shared behavior.

Refactoring Recommendations

  1. Extract shared builder helper
    • Create a reusable function (e.g., buildListSafeOutputJob) that accepts the job name/id, env key, outputs map, script getter, permissions, and default max. Use it in add-labels, assign-milestone, and assign-to-user.
    • Estimated effort: Medium (1–2 hours) to design helper + migrate three call sites.
    • Benefits: Single source of truth for env setup, defaults, and job wiring; easier to add future list-based jobs.
  2. Consolidate config parsing
    • Introduce a generic parseListJobConfig(outputMap, key, defaultMax) to handle ParseListJobConfig + parseBaseSafeOutputConfig + empty-config fallback. Call it from the three parsers instead of duplicating the logic.
    • Estimated effort: Low (30–45 minutes).

Implementation Checklist

  • Review duplication findings
  • Prioritize refactoring tasks
  • Create refactoring plan
  • Implement changes
  • Update tests
  • Verify no functionality broken

Analysis Metadata

  • Analyzed Files: 6 (.go builders/parsers; tests excluded)
  • Detection Method: Serena semantic code analysis
  • Commit:
  • Analysis Date: 2025-12-12T21:04:26Z

AI generated by Duplicate Code Detector

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions