Skip to content

Remove duplicate log statements in parseUpdateEntityConfig#28634

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-duplicate-log-statements
Draft

Remove duplicate log statements in parseUpdateEntityConfig#28634
Copilot wants to merge 2 commits intomainfrom
copilot/fix-duplicate-log-statements

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 26, 2026

parseUpdateEntityConfig logged identical messages to both updateEntityHelpersLog (file-level) and the caller-provided logger at the same call sites, doubling debug output on every update-issue, update-pull-request, update-discussion, etc. invocation.

Changes

  • Remove duplicate parse log: Drop updateEntityHelpersLog.Printf("Parsing %s configuration", ...) — the caller's logger.Printf already emits this with a more specific prefix.
  • Consolidate error log: Replace the two-line invalid-target-repo logs with a single logger.Printf that preserves the ConfigKey context previously only on the removed line.
  • Retain unique detail log: Keep updateEntityHelpersLog.Printf("Parsed target config for %s: target=%s", ...) — it has no counterpart in the caller logger.
// Before
updateEntityHelpersLog.Printf("Parsing %s configuration", params.ConfigKey)
logger.Printf("Parsing %s configuration", params.ConfigKey)

// After
logger.Printf("Parsing %s configuration", params.ConfigKey)
// Before
updateEntityHelpersLog.Printf("Invalid target-repo configuration for %s", params.ConfigKey)
logger.Print("Invalid target-repo configuration")

// After
logger.Printf("Invalid target-repo configuration for %s", params.ConfigKey)

Copilot AI changed the title [WIP] Fix duplicate log statements in parseUpdateEntityConfig Remove duplicate log statements in parseUpdateEntityConfig Apr 26, 2026
Copilot AI requested a review from gh-aw-bot April 26, 2026 20:57
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — great cleanup work on the duplicate log statements in parseUpdateEntityConfig! Removing the redundant updateEntityHelpersLog calls and consolidating the error message with the ConfigKey context is a nice, focused improvement.

One thing that would strengthen this PR before merge:

  • No test coverage — there are no test file changes included. Even a small unit test or table-driven test case for parseUpdateEntityConfig covering the invalid-target-repo path (and confirming the log output) would give reviewers confidence the consolidation doesn't silently drop information.

If you'd like a hand, here's a prompt you can assign to your coding agent:

Add or extend unit tests for `parseUpdateEntityConfig` in `pkg/workflow/update_entity_helpers.go`.
Cover the following scenarios:
1. Valid configuration — verify the function returns a non-nil *UpdateEntityConfig.
2. Invalid target-repo configuration (isInvalid == true) — verify the function returns nil and that logger receives the message "Invalid target-repo configuration for <ConfigKey>" (with the ConfigKey interpolated, not the old context-free string).
Use the existing test patterns in the `pkg/workflow/` package.

Generated by Contribution Check · ● 925.7K ·

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 log statements in parseUpdateEntityConfig (update_entity_helpers.go)

2 participants