Skip to content

refactor: introduce options structs to eliminate excessivefuncparams linter violations - #46065

Merged
pelikhan merged 15 commits into
mainfrom
copilot/deep-report-extract-download-artifacts-options-str
Jul 16, 2026
Merged

refactor: introduce options structs to eliminate excessivefuncparams linter violations#46065
pelikhan merged 15 commits into
mainfrom
copilot/deep-report-extract-download-artifacts-options-str

Conversation

CopilotAI commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Two production functions carried //nolint:excessivefuncparams suppressions (9 params each), and four sibling functions in logs_download.go repeated an identical 8-param signature. This introduces options structs to collapse all of them.

Changes

  • downloadArtifactsOptions (logs_download.go) — bundles (runID, outputDir, verbose, owner, repo, hostname, artifactFilter) shared by downloadArtifactsByName, retryCriticalArtifacts, downloadRunArtifacts, and ensureUsageAwInfoFallback
  • runArtifactsConcurrentOptions (logs_run_processor.go) — collapses 7 positional params after ctx/runs in downloadRunArtifactsConcurrent; removes //nolint:excessivefuncparams
  • updateActionsOptions (update_actions.go) — collapses 7 positional params after ctx/deps in updateActionsInWorkflowFiles; removes //nolint:excessivefuncparams

Call sites updated across all callers and tests. Public API (UpdateActionsInWorkflowFiles) is unchanged.

Before / After

// BeforedownloadRunArtifactsConcurrent(ctx, runs, outputDir, verbose, maxRuns, repoOverride, artifactFilter, evalsOnly, artifactSets) //nolint:excessivefuncparams// AfterdownloadRunArtifactsConcurrent(ctx, runs, runArtifactsConcurrentOptions{
outputDir: outputDir,
verbose: verbose,
maxRuns: maxRuns,
repoOverride: repoOverride,
// ...
})

Zero behavioral change.

CopilotAIand others added 8 commits July 16, 2026 19:16
…violations
- Add `downloadArtifactsOptions` struct in logs_download.go to collapse the
common (runID, outputDir, verbose, owner, repo, hostname, artifactFilter)
parameters shared by 4 sibling functions
- Refactor `downloadArtifactsByName`, `retryCriticalArtifacts`,
`downloadRunArtifacts`, and `ensureUsageAwInfoFallback` to use the struct
- Add `runArtifactsConcurrentOptions` struct in logs_run_processor.go and
refactor `downloadRunArtifactsConcurrent` (was 9 params → 3 params)
- Add `updateActionsOptions` struct in update_actions.go and refactor
`updateActionsInWorkflowFiles` (was 9 params → 3 params)
- Update all call sites across audit.go, audit_comparison.go, audit_diff.go,
forecast_compute.go, logs_orchestrator.go, logs_orchestrator_stdin.go,
logs_run_processor.go, outcomes_command.go, view_command.go and test files
- Removes 2 excessivefuncparams linter violations with zero behavioral change
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Per code review feedback:
- In downloadRunArtifacts: remove unpack of runID/outputDir/verbose/owner/repo/hostname
locals; access opts fields directly. Keep artifactFilter as a local since it
is narrowed to `missing` in the filtered download path.
- In updateActionsInWorkflowFiles: remove unpack of engineOverride/verbose/
disableReleaseBump/noCompile/coolDown/approve locals; access opts fields directly.
Keep workflowsDir as a local since it falls back to getWorkflowsDir() when empty.
- Simplify downloadArtifactsByName calls to pass opts directly instead of
reconstructing a struct literal with individual opts.X fields.
- Fix mangled comment and log format string where regex replaced variable names
inside string literals.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Per second round of code review feedback:
- In downloadRunArtifacts: remove the `artifactFilter` local that shadowed
opts.artifactFilter. Access opts.artifactFilter directly throughout the
function; where narrowing is needed, assign opts.artifactFilter = missing
directly (opts is a value copy, so this only affects the local scope).
- In updateActionsInWorkflowFiles: remove the `workflowsDir` local that
shadowed opts.workflowsDir. Assign opts.workflowsDir = getWorkflowsDir()
when empty and use opts.workflowsDir everywhere.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Since opts.artifactFilter is now used directly (narrowed via assignment),
retryCriticalArtifacts and ensureUsageAwInfoFallback can receive opts
directly rather than a reconstructed struct literal with the same fields.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Update doc comments on runArtifactsConcurrentOptions and updateActionsOptions
to accurately state '8-parameter positional list' instead of '7-parameter'.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ounts
Replace "N-parameter positional list" wording with "long positional parameter
list" to avoid ambiguity over whether ctx/deps/runs are counted.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ral in forecast_compute
- Move `opts` before `names` in downloadArtifactsByName signature (opts is the
common parameter, names is the function-specific one)
- Update all call sites accordingly
- In forecast_compute.go, extract the inline downloadArtifactsOptions struct
literal to a named local variable for readability
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Extract downloadArtifactsOptions struct and remove linter suppressionsrefactor: introduce options structs to eliminate excessivefuncparams linter violationsJul 16, 2026
CopilotAI requested a review from pelikhanJuly 16, 2026 19:33
@pelikhan
pelikhan marked this pull request as ready for review July 16, 2026 19:37
CopilotAI review requested due to automatic review settings July 16, 2026 19:37

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 several internal pkg/cli helpers to replace long positional parameter lists (and associated //nolint:excessivefuncparams suppressions) with small, per-call options structs, while keeping the exported API UpdateActionsInWorkflowFiles unchanged.

Changes:

  • Introduces downloadArtifactsOptions and updates artifact-download helpers/call sites to use it instead of repeating an 8-parameter signature.
  • Introduces runArtifactsConcurrentOptions to simplify downloadRunArtifactsConcurrent invocation and remove excessive-parameter lint suppression.
  • Introduces updateActionsOptions to simplify updateActionsInWorkflowFiles (internal) invocation and remove excessive-parameter lint suppression.
Show a summary per file
FileDescription
pkg/cli/view_command.goUpdates artifact download call to pass downloadArtifactsOptions.
pkg/cli/update_actions.goAdds updateActionsOptions and refactors internal workflow update function signature.
pkg/cli/update_actions_test.goUpdates tests to call updateActionsInWorkflowFiles with updateActionsOptions.
pkg/cli/outcomes_command.goUpdates artifact download call to pass downloadArtifactsOptions.
pkg/cli/logs_run_processor.goAdds runArtifactsConcurrentOptions and refactors concurrent artifact download entrypoint and its internal calls.
pkg/cli/logs_parallel_test.goUpdates parallel download tests to use runArtifactsConcurrentOptions.
pkg/cli/logs_orchestrator.goUpdates orchestrator call site to pass runArtifactsConcurrentOptions.
pkg/cli/logs_orchestrator_test.goUpdates orchestrator tests to use runArtifactsConcurrentOptions.
pkg/cli/logs_orchestrator_stdin.goUpdates stdin orchestrator call site to pass runArtifactsConcurrentOptions.
pkg/cli/logs_download.goIntroduces downloadArtifactsOptions and refactors artifact download helpers to consume it.
pkg/cli/logs_download_test.goUpdates download-related tests to use downloadArtifactsOptions.
pkg/cli/forecast_compute.goUpdates usage-artifact download helper to call downloadArtifactsByName via downloadArtifactsOptions.
pkg/cli/audit.goUpdates audit artifact download calls to pass downloadArtifactsOptions.
pkg/cli/audit_test.goUpdates audit tests to call downloadRunArtifacts via downloadArtifactsOptions.
pkg/cli/audit_diff.goUpdates audit diff artifact download call to pass downloadArtifactsOptions.
pkg/cli/audit_comparison.goUpdates baseline artifact download call to pass downloadArtifactsOptions.

Review details

Tip

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

  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines 739 to 741
} else {
logsDownloadLog.Printf("Could not list artifacts (will use bulk download): %v", listErr)
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Fixed in 45a5164. When artifact listing fails and a filter is set, downloadRunArtifacts now falls back to trying the requested filter names directly instead of returning ErrNoArtifacts. Added a unit test (TestDownloadRunArtifacts_FilterFallsBackToRequestedNamesWhenListFails) to cover this path.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Jul 16, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot run pr-finisher skill

Ran pr-finisher workflow and applied the in-scope reviewer fix in commit 45a5164. I also triaged checks/mergeability and verified local validation; CI on the new head will require a maintainer re-trigger.

CopilotAIand others added 4 commits July 16, 2026 21:47
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan
pelikhan merged commit 8867a24 into mainJul 16, 2026
29 checks passed
@pelikhan
pelikhan deleted the copilot/deep-report-extract-download-artifacts-options-str branch July 16, 2026 22:10
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.12

Sign up for freeto 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.

[deep-report] [Code Quality] Extract downloadArtifactsOptions struct + remove 2 nolint:excessivefuncparams suppressions in logs_download

3 participants

@pelikhan