Skip to content

Consolidate duplicate config parsing functions with generic helper#3490

Merged
pelikhan merged 4 commits into
mainfrom
copilot/consolidate-config-parsing-functions
Nov 8, 2025
Merged

Consolidate duplicate config parsing functions with generic helper#3490
pelikhan merged 4 commits into
mainfrom
copilot/consolidate-config-parsing-functions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 8, 2025

parseTitlePrefixFromConfig and parseTargetRepoFromConfig had ~90% code duplication, differing only in key names and log messages.

Changes

  • Added parseStringFromConfig(configMap, key) - generic helper implementing the common pattern: existence check → type assertion → logging
  • Refactored both functions to delegate to the helper (14 lines → 2 lines total)
  • Added 8 test cases covering edge cases (nil values, type mismatches, special characters)

Example

Before:

func parseTitlePrefixFromConfig(configMap map[string]any) string {
    if titlePrefix, exists := configMap["title-prefix"]; exists {
        if titlePrefixStr, ok := titlePrefix.(string); ok {
            configLog.Printf("Parsed title-prefix from config: %s", titlePrefixStr)
            return titlePrefixStr
        }
    }
    return ""
}

After:

func parseTitlePrefixFromConfig(configMap map[string]any) string {
    return parseStringFromConfig(configMap, "title-prefix")
}

Future config fields can now use the same helper instead of duplicating the pattern.

Fixes #3481

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Consolidate duplicate config parsing functions in config.go</issue_title>
<issue_description>## Objective
Create a generic helper function to eliminate near-duplicate config parsing functions in pkg/workflow/config.go.

Context

The analysis identified that parseTitlePrefixFromConfig and parseTargetRepoFromConfig have ~90% code similarity, differing only in the key name, variable names, and log message text. This is Issue #1 from the semantic function clustering analysis (Priority 1: High Impact, Low Effort).

Related to #3478

Approach

  1. Create a new generic helper function parseStringFromConfig(configMap map[string]any, key string) string
  2. Implement the common pattern: check existence, type assertion, logging
  3. Refactor parseTitlePrefixFromConfig to use the helper
  4. Refactor parseTargetRepoFromConfig to use the helper
  5. Ensure all existing tests continue to pass

Files to Modify

  • pkg/workflow/config.go - Add generic helper and refactor existing functions
  • pkg/workflow/config_test.go - Add tests for the new helper function (if file exists, or create it)

Acceptance Criteria

  • Generic parseStringFromConfig helper function created
  • parseTitlePrefixFromConfig refactored to use the helper
  • parseTargetRepoFromConfig refactored to use the helper
  • All existing tests pass
  • New tests cover the generic helper function
  • Code duplication reduced from ~90% to minimal
  • Logging behavior preserved

Estimated Effort

30 minutes

Benefits

AI generated by Plan Command for #3478</issue_description>

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


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Created generic parseStringFromConfig helper function
- Refactored parseTitlePrefixFromConfig to use the helper
- Refactored parseTargetRepoFromConfig to use the helper
- Added comprehensive tests for parseStringFromConfig
- All existing tests pass, maintaining backward compatibility
- Reduced code duplication from ~90% to minimal

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Consolidate duplicate config parsing functions in config.go Consolidate duplicate config parsing functions with generic helper Nov 8, 2025
Copilot AI requested a review from pelikhan November 8, 2025 17:08
@pelikhan pelikhan marked this pull request as ready for review November 8, 2025 19:01
Copilot AI review requested due to automatic review settings November 8, 2025 19:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors config parsing helpers by extracting a generic parseStringFromConfig function to reduce code duplication. The refactoring consolidates the logic for extracting and validating string values from configuration maps.

  • Introduced a new generic helper parseStringFromConfig that extracts string values from config maps
  • Refactored parseTitlePrefixFromConfig and parseTargetRepoFromConfig to delegate to the new helper
  • Added comprehensive unit tests for the new helper function covering all edge cases

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/workflow/config.go Introduced parseStringFromConfig helper and refactored two existing functions to use it
pkg/workflow/config_parsing_helpers_test.go Added comprehensive test coverage for the new parseStringFromConfig helper function

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/workflow/config.go Outdated
pelikhan and others added 2 commits November 8, 2025 11:25
@pelikhan pelikhan merged commit ca7d2ce into main Nov 8, 2025
8 checks passed
@pelikhan pelikhan deleted the copilot/consolidate-config-parsing-functions branch November 8, 2025 20:39
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.

[task] Consolidate duplicate config parsing functions in config.go

3 participants