Skip to content

[fp-enhancer] Improve pkg/cli with functional patterns#14751

Closed
github-actions[bot] wants to merge 1 commit intomainfrom
main-66977b299017b6ec
Closed

[fp-enhancer] Improve pkg/cli with functional patterns#14751
github-actions[bot] wants to merge 1 commit intomainfrom
main-66977b299017b6ec

Conversation

@github-actions
Copy link
Contributor

Functional/Immutability Enhancements - Package: pkg/cli

This PR applies moderate, tasteful functional/immutability techniques to the pkg/cli package to improve code clarity, safety, testability, and maintainability.

Round-Robin Progress: This is part of a systematic package-by-package refactoring. Next package to process: pkg/console

Note: Cache update attempted but gh-aw extension not available in this environment. Next run should start with pkg/console.

Summary of Changes

1. Immutability Improvements

  • 1 variable converted from mutable to immutable initialization
  • 1 slice pre-allocated instead of incremental building

Files affected:

  • pkg/cli/actionlint.go - Pre-allocated slice for relative paths (lines 197-205)

2. Transformative Data Operations

  • 2 imperative loops converted to functional transformations
  • Applied existing sliceutil.FilterMap and sliceutil.Map utilities

Files affected:

  • pkg/cli/actions_build_command.go - Replaced directory filtering loop with sliceutil.FilterMap (lines 140-143)
  • pkg/cli/add_workflow_pr.go - Replaced workflow name extraction loop with sliceutil.Map (lines 85-87)

Benefits

  • Safety: Pre-allocated slice eliminates unnecessary append operations and potential reallocation overhead
  • Clarity: Functional transformations make intent explicit (filter-and-map vs imperative loop)
  • Maintainability: Leverages existing sliceutil package for consistency
  • Performance: Pre-allocation reduces memory allocation overhead for known-size slices

Principles Applied

  1. Immutability First: Pre-allocate slices when size is known rather than building incrementally
  2. Declarative Over Imperative: Use functional utilities to express "what" not "how"
  3. Leverage Existing Utilities: Use sliceutil package functions for consistency
  4. Pragmatic Balance: Changes improve clarity without dogmatic adherence

Testing

  • ✅ All tests pass (TestParseAndDisplayActionlintOutput, TestGetActionDirectories)
  • ✅ Test existence verified BEFORE refactoring (via code search)
  • ✅ Formatting passes (make fmt)
  • ✅ No behavioral changes - functionality is identical
  • ✅ Manual diff review confirms only style improvements

Test output:

=== RUN   TestParseAndDisplayActionlintOutput
--- PASS: TestParseAndDisplayActionlintOutput (0.00s)
=== RUN   TestGetActionDirectories  
--- PASS: TestGetActionDirectories (0.00s)
PASS
ok      github.com/github/gh-aw/pkg/cli 0.021s

Review Focus

Please verify:

  • Pre-allocation pattern in actionlint.go is appropriate
  • sliceutil.FilterMap usage in actions_build_command.go improves clarity
  • sliceutil.Map usage in add_workflow_pr.go is consistent with codebase style
  • No unintended side effects or behavior changes

Examples

Before: Mutable slice building in actionlint.go

var relPaths []string
for _, lockFile := range lockFiles {
    relPath, err := filepath.Rel(gitRoot, lockFile)
    if err != nil {
        return fmt.Errorf("failed to get relative path for %s: %w", lockFile, err)
    }
    relPaths = append(relPaths, relPath)
}

After: Pre-allocated immutable initialization

relPaths := make([]string, len(lockFiles))
for i, lockFile := range lockFiles {
    relPath, err := filepath.Rel(gitRoot, lockFile)
    if err != nil {
        return fmt.Errorf("failed to get relative path for %s: %w", lockFile, err)
    }
    relPaths[i] = relPath
}

Before: Imperative directory filtering in actions_build_command.go

var dirs []string
for _, entry := range entries {
    if entry.IsDir() {
        dirs = append(dirs, entry.Name())
    }
}

After: Functional filter-and-map

dirs := sliceutil.FilterMap(entries,
    func(entry os.DirEntry) bool { return entry.IsDir() },
    func(entry os.DirEntry) string { return entry.Name() },
)

Before: Imperative workflow name extraction in add_workflow_pr.go

workflowNames := make([]string, len(workflows))
for i, wf := range workflows {
    workflowNames[i] = wf.WorkflowName
}

After: Functional map transformation

workflowNames := sliceutil.Map(workflows, func(wf *WorkflowSpec) string {
    return wf.WorkflowName
})

Impact

  • 3 files modified with surgical, minimal changes
  • ~15 lines of code replaced with clearer functional patterns
  • Zero behavioral changes - all tests pass unchanged
  • Consistent with existing codebase - uses existing sliceutil package

Automated by Functional Pragmatist - applying moderate functional/immutability techniques to pkg/cli

AI generated by Functional Pragmatist

  • expires on Feb 11, 2026, 9:30 AM UTC

- Pre-allocate slice in actionlint.go for known-size data
- Use sliceutil.FilterMap in actions_build_command.go
- Use sliceutil.Map in add_workflow_pr.go for workflow name extraction

Benefits:
- Reduces unnecessary append operations and potential reallocations
- Makes data transformations more explicit and declarative
- Leverages existing sliceutil utilities for consistency
@github-actions
Copy link
Contributor Author

🔍 PR Triage Results

Category: refactor | Risk: low | Priority: 32/100

Scores Breakdown

  • Impact: 25/50 - Code quality improvement through functional patterns
  • Urgency: 0/30 - Recent PR (3.0 hours old), no urgent need
  • Quality: 7/20 - Draft status, reduced quality score, pending CI

📋 Recommended Action: batch_review

Rationale: Applies functional programming patterns to pkg/cli (pre-allocated slices, FilterMap). Low impact changes (~27 lines) but marked as draft.

Next Steps:

  • Review functional pattern implementations
  • Ensure no behavior changes
  • Mark as ready for review when complete

Triaged by PR Triage Agent on 2026-02-10

AI generated by PR Triage Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant