Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 20, 2025

Four update entity parsers (update_issue, update_pull_request, update_discussion, update_release) contained identical scaffolding for parsing base config and entity-specific fields, creating maintenance burden and drift risk.

Changes

  • New helper: parseUpdateEntityConfigWithFields() accepts field specs and optional custom parser, eliminating ~40 lines of duplication
  • Declarative field specs: UpdateEntityFieldSpec defines field name, parsing mode, and destination pointer
  • Simplified parsers: All four parsers now use the helper, reducing to declarative field specifications

Before/After

Before:

baseConfig, configMap := c.parseUpdateEntityBase(outputMap, UpdateEntityIssue, "update-issue", updateIssueLog)
if baseConfig == nil { return nil }
cfg := &UpdateIssuesConfig{UpdateEntityConfig: *baseConfig}
cfg.Status = parseUpdateEntityBoolField(configMap, "status", FieldParsingKeyExistence)
cfg.Title = parseUpdateEntityBoolField(configMap, "title", FieldParsingKeyExistence)
cfg.Body = parseUpdateEntityBoolField(configMap, "body", FieldParsingKeyExistence)

After:

cfg := &UpdateIssuesConfig{}
baseConfig, _ := c.parseUpdateEntityConfigWithFields(outputMap, UpdateEntityParseOptions{
    EntityType: UpdateEntityIssue,
    ConfigKey:  "update-issue",
    Logger:     updateIssueLog,
    Fields: []UpdateEntityFieldSpec{
        {Name: "status", Mode: FieldParsingKeyExistence, Dest: &cfg.Status},
        {Name: "title", Mode: FieldParsingKeyExistence, Dest: &cfg.Title},
        {Name: "body", Mode: FieldParsingKeyExistence, Dest: &cfg.Body},
    },
})
if baseConfig == nil { return nil }
cfg.UpdateEntityConfig = *baseConfig

Special Cases

Discussion parser uses CustomParser callback for AllowedLabels handling - demonstrating extensibility for entity-specific logic without duplicating base parsing.

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Update entity config parsing duplicated across update_issue/update_pull_request/update_discussion/update_release</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Update entity config parsing

Analysis of commit daf6f5e

Assignee: @copilot

Summary

The update entity configuration parsers repeat the same base-config extraction and bool-field wiring across multiple files, creating parallel implementations for issues, pull requests, discussions, and releases.

Duplication Details

Pattern: Repeated parseUpdate*Config scaffolding around parseUpdateEntityBase

  • Severity: Medium
  • Occurrences: 4
  • Locations:
    • pkg/workflow/update_issue.go:17-35
    • pkg/workflow/update_pull_request.go:16-35
    • pkg/workflow/update_discussion.go:18-48
    • pkg/workflow/update_release.go:14-27
  • Code Sample:
// Representative snippet (update_issue.go)
baseConfig, configMap := c.parseUpdateEntityBase(outputMap, UpdateEntityIssue, "update-issue", updateIssueLog)
if baseConfig == nil {
    return nil
}
cfg := &UpdateIssuesConfig{UpdateEntityConfig: *baseConfig}
cfg.Status = parseUpdateEntityBoolField(configMap, "status", FieldParsingKeyExistence)

Impact Analysis

  • Maintainability: Four nearly identical parsers require parallel edits when behavior changes, increasing risk of drift.
  • Bug Risk: Inconsistent flag parsing (key-existence vs bool) can emerge silently if one parser is updated without the others.
  • Code Bloat: Redundant scaffolding adds noise around the actual entity-specific logic.

Refactoring Recommendations

  1. Extract shared parser helper
    • Create a table-driven helper (e.g., parseUpdateEntityConfig(outputMap, entityType, key, log, fields)), where fields map names to parsing mode, returning the base config plus parsed flags.
    • Estimated effort: 2-3 hours; reduces four functions to concise wrappers.
    • Benefits: Single place to adjust parsing defaults and logging.
  2. Align field parsing strategies
    • Consolidate the choice between FieldParsingBoolValue and FieldParsingKeyExistence in the helper with per-field options to prevent divergence.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 4
  • Detection Method: Serena semantic code analysis
  • Commit: daf6f5e
  • Analysis Date: 2025-12-20T19:28:59Z

AI generated by Duplicate Code Detector

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate entity config parsing Consolidate update entity config parsing with table-driven helper Dec 20, 2025
Copilot AI requested a review from mnkiefer December 20, 2025 21:11
@pelikhan pelikhan marked this pull request as ready for review December 20, 2025 21:31
@pelikhan pelikhan merged commit c52a4b9 into main Dec 20, 2025
162 checks passed
@pelikhan pelikhan deleted the copilot/refactor-entity-config-parsing branch December 20, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Update entity config parsing duplicated across update_issue/update_pull_request/update_discussion/update_release

3 participants