Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 19, 2025

Four config parsers (update-issue, update-pull-request, update-release, update-discussion) duplicated ~15 lines of identical scaffolding: building UpdateEntityJobParams, calling parseUpdateEntityConfig, nil checking, and extracting the config map.

Changes

  • Added parseUpdateEntityBase() helper in update_entity_helpers.go

    • Consolidates common scaffolding into single reusable function
    • Returns base config and config map for entity-specific parsing
  • Refactored all four parsers to use the helper

    • update_issue.go: 62 → 51 lines (-18%)
    • update_pull_request.go: 61 → 51 lines (-16%)
    • update_release.go: 32 → 28 lines (-13%)
    • update_discussion.go: 71 → 60 lines (-15%)

Before/After

// Before: Each parser repeated this pattern
func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssuesConfig {
    params := UpdateEntityJobParams{
        EntityType: UpdateEntityIssue,
        ConfigKey:  "update-issue",
    }
    parseSpecificFields := func(configMap map[string]any, baseConfig *UpdateEntityConfig) {}
    baseConfig := c.parseUpdateEntityConfig(outputMap, params, updateIssueLog, parseSpecificFields)
    if baseConfig == nil {
        return nil
    }
    updateIssuesConfig := &UpdateIssuesConfig{UpdateEntityConfig: *baseConfig}
    
    // Then extract config map again from outputMap...
    if configData, exists := outputMap["update-issue"]; exists {
        if configMap, ok := configData.(map[string]any); ok {
            // Parse entity-specific fields
        }
    }
    return updateIssuesConfig
}

// After: Single helper call
func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssuesConfig {
    baseConfig, configMap := c.parseUpdateEntityBase(outputMap, UpdateEntityIssue, "update-issue", updateIssueLog)
    if baseConfig == nil {
        return nil
    }
    updateIssuesConfig := &UpdateIssuesConfig{UpdateEntityConfig: *baseConfig}
    
    if configMap != nil {
        // Parse entity-specific fields
    }
    return updateIssuesConfig
}

Total: 36 lines of duplication eliminated, +41 lines for reusable helper. Future update entities follow established pattern with minimal boilerplate.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/user
    • Triggering command: `/usr/bin/gh gh api user --jq .login ithub/workflows pkg/workflow/strict_mode_deprecated_test.go /opt/hostedtoolcache/node/24.12.0/x64/bin/git pkg/workflow/strgit pkg/workflow/strrev-parse pkg/workflow/str--git-dir git comm�� ithub/workflows Refactor update entity config parsers to eliminate duplication

-ifaceassert /bin/sh ows nly modules/@npmcli/--show-toplevel /bin/sh` (http block)

  • Triggering command: /usr/bin/gh gh api user --jq .login om/different-ownremote.origin.url --others /usr/bin/bash ale-repos/git/regit config /usr/lib/git-cor--show-toplevel bash --no�� -node.git -node.git /opt/hostedtoolcache/go/1.25.0/x64/bin/go https://github.cinfocmp REDACTED /usr/bin/git go (http block)
  • Triggering command: /usr/bin/gh gh api user --jq .login -dotnet.git origin 56a90e4205519e234f4bf6ac36511166170/log.json /home/REDACTED/worinfocmp rev-parse /usr/bin/gh /opt/hostedtoolcache/go/1.25.0/x--no-legend -V=f�� ithub/workflows gh (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Go update-entity config parsing duplicated across issue/PR/release</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Update entity config parsers

Analysis of commit a2389f6

Assignee: @copilot

Summary

Three separate config parsers for update-issue, update-pull-request, and update-release replicate the same setup/tear-down logic (building UpdateEntityJobParams, calling parseUpdateEntityConfig, and re-wrapping the result) with minor per-entity tweaks. The shared scaffolding is ~15+ lines repeated, increasing maintenance overhead when adding new update entities or changing parsing behavior.

Duplication Details

Pattern: Repeated update config scaffolding

  • Severity: Medium
  • Occurrences: 3
  • Locations:
    • pkg/workflow/update_issue.go:17-61
    • pkg/workflow/update_pull_request.go:16-61
    • pkg/workflow/update_release.go:14-31
  • Code Sample:
params := UpdateEntityJobParams{EntityType: UpdateEntityIssue, ConfigKey: "update-issue"}
baseConfig := c.parseUpdateEntityConfig(outputMap, params, updateIssueLog, parseSpecificFields)
if baseConfig == nil {
    return nil
}
updateIssuesConfig := &UpdateIssuesConfig{UpdateEntityConfig: *baseConfig}

(Same scaffold appears in update-pull-request and update-release with only names changed.)

Impact Analysis

  • Maintainability: Shared logic must be edited in three places when adding defaults, logging, or validation changes for update entities.
  • Bug Risk: Subtle differences (e.g., logging, nil handling) can diverge across entity types, causing inconsistent parsing or silent failures.
  • Code Bloat: Extra boilerplate obscures the per-entity differences (fields and toggles) and makes onboarding harder.

Refactoring Recommendations

  1. Extract a generic helper (e.g., parseUpdateEntity(outputMap, params, logger, parseFields) or table-driven config) returning the typed config struct to remove repeated scaffolding.
    • Extract common setup (UpdateEntityJobParams, parseUpdateEntityConfig call, nil check) into one reusable function.
    • Estimated effort: 1-2h; benefits: single point of change for all update entity parsers.
  2. Encapsulate per-entity field parsing via callbacks or a small descriptor map (e.g., booleans for status, title, body) to reduce bespoke code paths.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 3
  • Detection Method: Serena semantic code analysis
  • Commit: a2389f6
  • Analysis Date: 2025-01-06

AI generated by Duplicate Code Detector

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


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Added parseUpdateEntityBase helper to handle common scaffolding
- Refactored parseUpdateIssuesConfig to use new helper (reduced from 62 to 51 lines)
- Refactored parseUpdatePullRequestsConfig to use new helper (reduced from 61 to 51 lines)
- Refactored parseUpdateReleaseConfig to use new helper (reduced from 32 to 28 lines)
- Refactored parseUpdateDiscussionsConfig to use new helper (reduced from 71 to 60 lines)
- Eliminated ~15+ lines of duplicate scaffolding per parser
- All existing tests pass - no functionality changes

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicated config parsing for update entities Refactor update entity config parsers to eliminate duplicate scaffolding Dec 19, 2025
Copilot AI requested a review from mnkiefer December 19, 2025 23:15
@pelikhan pelikhan marked this pull request as ready for review December 19, 2025 23:31
@pelikhan pelikhan merged commit 2ada71b into main Dec 19, 2025
4 checks passed
@pelikhan pelikhan deleted the copilot/refactor-update-entity-parsers branch December 19, 2025 23:31
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] Go update-entity config parsing duplicated across issue/PR/release

3 participants