[github-actions]

Code Simplification - 2026-05-13

This PR simplifies recently modified code to improve clarity, consistency, and maintainability while preserving all functionality.

Files Simplified

• pkg/workflow/reactions.go - Extracted shared helper to eliminate three identical boolean-field parsing blocks

Improvements Made

Reduced Duplication (DRY)

The parseReactionConfig function contained three nearly identical blocks for parsing the issues, pull-requests, and discussions boolean fields:

// Before (repeated 3 times with different field names)
reactionIssues := true
if issuesValue, hasIssues := reactionMap["issues"]; hasIssues {
    issuesBool, ok := issuesValue.(bool)
    if !ok {
        return ..., fmt.Errorf("reaction.issues must be a boolean value, got %T", issuesValue)
    }
    reactionIssues = issuesBool
}

Extracted into a shared helper:

// After - single helper called 3 times
func parseBoolReactionField(m map[string]any, key string) (bool, error) {
    v, ok := m[key]
    if !ok {
        return true, nil  // absent == enabled
    }
    b, ok := v.(bool)
    if !ok {
        return false, fmt.Errorf("reaction.%s must be a boolean value, got %T", key, v)
    }
    return b, nil
}

Changes Based On

Recent changes from:

• Commit 22cc7fb - Add immediate centralized command reactions and propagate desired reaction context (Add immediate centralized command reactions and propagate desired reaction context #31847)

Testing

• ✅ All reaction tests pass (go test -run TestReaction ./pkg/workflow/)
• ✅ Build succeeds (make build)
• ✅ Formatting passes (make fmt)
• ✅ No functional changes - behaviour is identical
• i️ Two pre-existing test failures (TestCommentOutProcessedFieldsInOnSection, TestWasmGolden_CompileFixtures) exist on main and are unrelated to this change

Review Focus

Please verify:

• The helper correctly defaults to true when the key is absent
• Error messages remain consistent with the original (field name is interpolated via key)
• No functional changes to parseReactionConfig

---

Automated by Code Simplifier Agent - analyzing code from the last 24 hours

Generated by Code Simplifier · ● 14.4M · ◷

• expires on May 14, 2026, 4:42 AM UTC

[pelikhan]

/q optimize

[github-actions]

🎩 Mission equipment ready! Q has optimized your workflow. Use wisely, 007! 🔫

[Copilot]

Pull request overview

This PR refactors parseReactionConfig by extracting the repeated boolean-field parsing logic for reaction target flags into a shared helper, aiming to reduce duplication while preserving behavior.

Changes:

• Replaced three duplicated parsing blocks (issues, pull-requests, discussions) with calls to a new parseBoolReactionField helper.
• Added parseBoolReactionField to centralize defaulting-to-true behavior and consistent type error reporting.

Show a summary per file

File	Description
pkg/workflow/reactions.go	Extracts a helper to DRY up boolean target parsing in parseReactionConfig while keeping the same defaults and error behavior.

Copilot's findings

Tip

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

• Files reviewed: 1/1 changed files
• Comments generated: 0

[github-actions]

Skills-Based Review 🧠

Applied /tdd based on the refactor nature of this PR — extracting a shared helper warrants verifying the helper's contract is tested directly.

Key Themes

• Missing unit test for the new helper: parseBoolReactionField is now the single source of truth for all three boolean fields, but it has no dedicated unit test. The behaviour (absent→true, non-bool→error) is currently only verified indirectly through parseReactionConfig integration paths.

Positive Highlights

• ✅ Excellent DRY refactor — three near-identical 7-line blocks collapsed to three 3-line call sites
• ✅ Helper signature is minimal and easy to reason about
• ✅ Error messages preserve field-name interpolation, keeping them identical to the originals
• ✅ Default-true-on-absence semantics are clearly documented in the function comment
• ✅ PR description is thorough and the testing checklist shows the author verified behaviour was preserved

Verdict

COMMENT — The refactor is clean and correct. The one suggestion (add a small unit test for parseBoolReactionField) is a low-effort improvement that would make the helper's contract self-documenting. Not blocking.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 3.7M

[github-actions]

[/tdd] parseBoolReactionField is a newly extracted helper with its own logic (absent-key default, type assertion, error formatting), but it has no dedicated unit tests. Since it is now the single authoritative source for all three boolean fields, a small table-driven test would make the contract explicit and guard against future drift.

Consider adding to reactions_test.go:

func TestParseBoolReactionField(t *testing.T) {
    tests := []struct {
        name    string
        m       map[string]any
        key     string
        want    bool
        wantErr bool
    }{
        {"absent key defaults to true", map[string]any{}, "issues", true, false},
        {"explicit true",  map[string]any{"issues": true},  "issues", true,  false},
        {"explicit false", map[string]any{"issues": false}, "issues", false, false},
        {"non-bool value", map[string]any{"issues": "yes"}, "issues", false, true},
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            got, err := parseBoolReactionField(tt.m, tt.key)
            if tt.wantErr {
                assert.Error(t, err, "expected error for non-bool value")
            } else {
                assert.NoError(t, err)
                assert.Equal(t, tt.want, got)
            }
        })
    }
}

This keeps the helper's semantics explicit and protects against accidental behaviour changes.

[github-actions]

Issue created: #31857

🎩 Equipped by Q