Refactor logs download orchestration to reduce pkg/cli largefunc backlog#46125
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pkg/cli largefunc backlog
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Great work on this refactor! 🎉 The decomposition of The PR includes targeted unit coverage for the extracted batch-sizing and empty-batch pagination logic, and the description clearly explains what changed and why. This looks ready for review!
|
There was a problem hiding this comment.
Pull request overview
Refactors workflow-log download orchestration into focused internal helpers while preserving the public entry point.
Changes:
- Extracts setup, collection, pagination, parsing, and finalization helpers.
- Adds unit tests for batch sizing and empty-batch pagination.
- Simplifies
DownloadWorkflowLogs.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/logs_orchestrator.go |
Retains high-level orchestration entry point. |
pkg/cli/logs_orchestrator_download.go |
Adds extracted download pipeline helpers. |
pkg/cli/logs_orchestrator_unit_test.go |
Tests extracted pagination and batch-size logic. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Medium
🤖 PR Triage
Rationale: DRAFT — pkg/cli log download orchestration refactor for largefunc backlog. No CI yet. Low urgency; defer until undrafted.
|
|
@copilot please run the Unresolved review threads:
Failed checks:
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in 08dcad2.
I can’t trigger a fresh CI run from this cloud-agent push path; a maintainer re-trigger is needed (close/reopen PR, workflow_dispatch, or maintainer push). |
|
@copilot please run the
|
Confirmed: there are no remaining open review threads on this PR; the prior thread is resolved and already covered by 08dcad2, so no additional code fix was needed. I re-triaged checks and inspected run/job logs:
I can’t re-run checks from this cloud-agent push path; a maintainer needs to re-trigger CI (close/reopen PR, workflow_dispatch, or maintainer push) for readiness verification on current head. |
|
@copilot please run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
I ran the
From this cloud-agent environment I can’t trigger fresh CI runs; a maintainer needs to re-trigger checks (close/reopen PR, workflow_dispatch, or maintainer push) to verify green status on the latest head. |
This issue tracks function-length reduction across
pkg/cliandpkg/workflowwith behavior-preserving refactors. This change takes a small slice of thepkg/clilogs pipeline by splittingDownloadWorkflowLogsinto focused orchestration helpers.What changed
DownloadWorkflowLogsbody into a dedicated helper file:pkg/cli/logs_orchestrator_download.goDownloadWorkflowLogsas the public entrypoint, but reduced it to high-level flow control:Helper extraction
--afterBehavior preserved
Targeted unit coverage
Example