refactor: Use IOStreams for command output instead of os.Stdout and log by kaizakin · Pull Request #372 · shipwright-io/cli

kaizakin · 2026-02-23T08:09:50Z

Changes

Fixes #90

The original issue was about inconsistent user facing logging.
Turns out 95% of the codebase is already following the best practice as the same as kubectl.

Following @IrvingMg's suggestion,
I've updated some inconsistencies to follow the same structure as the rest of the commands.

/kind feature

Submitter Checklist

Includes tests if functionality changed/was added
Includes docs if changes are user-facing
Set a kind label on this PR
Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Update user facing logs to follow the same consistent pattern

Copilot

Pull request overview

This PR standardizes user-facing CLI output by routing it through Kubernetes IOStreams (instead of writing directly to os.Stdout / fmt.Printf / log.Printf), aligning subcommands with the CLI’s existing stream-injection pattern.

Changes:

Update version subcommand to accept IOStreams and write to ioStreams.Out.
Switch multiple list subcommands’ tabwriter output from os.Stdout to io.Out.
Replace log.Printf/os.Stdout output in build upload with fmt.Fprintf(ioStreams.Out, ...) and thread streams through helpers.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/shp/cmd/version/version.go	Accepts `IOStreams` and writes version output to `ioStreams.Out`.
pkg/shp/cmd/root.go	Passes `ioStreams` into the `version` command constructor.
pkg/shp/cmd/clusterbuildstrategy/list.go	Writes tabular list output to `io.Out` instead of `os.Stdout`.
pkg/shp/cmd/buildstrategy/list.go	Writes tabular list output to `io.Out` instead of `os.Stdout`.
pkg/shp/cmd/buildrun/list.go	Writes tabular list output to `io.Out` instead of `os.Stdout`.
pkg/shp/cmd/build/upload.go	Uses `IOStreams` for user-facing progress/info output during buildrun creation and streaming.

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

pkg/shp/cmd/root.go

pkg/shp/cmd/build/upload.go

kaizakin · 2026-03-03T18:29:15Z

Verify check failing because of gosec, adding //nolint:gosec to suppress them

kaizakin · 2026-03-03T18:30:34Z

Ready to test again @adambkaplan

IrvingMg · 2026-03-04T09:34:55Z

/ok-to-test

IrvingMg · 2026-03-04T09:56:56Z

pkg/shp/cmd/build/upload.go

 // Run executes the primary business logic of this subcommand, by starting to watch over the build
 // pod status and react accordingly.
 func (u *UploadCommand) Run(p *params.Params, ioStreams *genericclioptions.IOStreams) error {
+	u.ioStreams = ioStreams


Do we need this? Wasn’t u.ioStreams initialized before?

we need that initialization @IrvingMg

u.ioStreams is only declared in the struct and never initialized before Run()

in Complete(), the second parameter is _ *genericclioptions.IOStreams and is intentionally ignored, so u.ioStreams is still nil there.

Methods like createBuildRun and performDataStreaming use u.ioStreams, and they are called from Run

I see. Thanks for checking.

Then I’d just make a small suggestion: move u.ioStreams = ioStreams to Complete, as that method seems more appropriate for initializing the fields.

Yup, Thanks for the review @IrvingMg

suggested changes has been made and ready for review again 😄

Signed-off-by: karthik balasubramanian <karthikbalasubramanian08@gmail.com>

IrvingMg

As I mentioned in #90 (comment), it looks like genericclioptions.IOStreams is the common approach for handling output in CLIs. Since shp already uses it in most commands, these changes look good to me.

Still, it would be great to hear feedback from other @shipwright-io/contributors.

/lgtm

SaschaSchwarze0 · 2026-03-09T14:05:19Z

pkg/shp/cmd/buildrun/list.go

 	//       find out more in kubectl libraries and use them

-	writer := tabwriter.NewWriter(os.Stdout, 0, 8, 2, '\t', 0)
+	writer := tabwriter.NewWriter(io.Out, 0, 8, 2, '\t', 0)


Can you rename io to ioStreams as in upload.go. I think we should not use variable names that match Golang library packages.

Same in other files.

I have made the requested changes and it's ready for review @SaschaSchwarze0

Signed-off-by: karthik balasubramanian <karthikbalasubramanian08@gmail.com>

SaschaSchwarze0

/approve
/lgtm

openshift-ci · 2026-03-10T07:28:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [SaschaSchwarze0]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note labels Feb 23, 2026

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 23, 2026

shipwright-ci-bot added this to Issues Feb 23, 2026

kaizakin force-pushed the feat/newlogger branch from c0b2e22 to bfe75a8 Compare February 23, 2026 11:49

kaizakin force-pushed the feat/newlogger branch from bfe75a8 to d87b5a2 Compare March 2, 2026 12:58

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 2, 2026

kaizakin marked this pull request as ready for review March 2, 2026 12:58

Copilot AI review requested due to automatic review settings March 2, 2026 12:58

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 2, 2026

openshift-ci bot requested review from adambkaplan and rxinui March 2, 2026 12:58

Copilot started reviewing on behalf of kaizakin March 2, 2026 12:59 View session

openshift-ci bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note labels Mar 2, 2026

Copilot AI reviewed Mar 2, 2026

View reviewed changes

pkg/shp/cmd/root.go Show resolved Hide resolved

pkg/shp/cmd/build/upload.go Show resolved Hide resolved

pkg/shp/cmd/build/upload.go Outdated Show resolved Hide resolved

kaizakin changed the title ~~Introduce New logger for centralized loging~~ refactor: Use IOStreams for command output instead of os.Stdout and log Mar 2, 2026

kaizakin force-pushed the feat/newlogger branch from d87b5a2 to 7b011f7 Compare March 2, 2026 13:10

pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 2, 2026

openshift-ci bot added release-note and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 2, 2026

kaizakin force-pushed the feat/newlogger branch from 7b011f7 to 4c88d32 Compare March 3, 2026 18:29

kaizakin force-pushed the feat/newlogger branch from 4c88d32 to a75d7c4 Compare March 3, 2026 18:33

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 3, 2026

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 3, 2026

kaizakin force-pushed the feat/newlogger branch from a75d7c4 to 20b0774 Compare March 3, 2026 18:50

pull-request-size bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 3, 2026

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 3, 2026

pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 3, 2026

openshift-ci bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 4, 2026

IrvingMg reviewed Mar 4, 2026

View reviewed changes

refactor: use IOStreams for command output instead of os.Stdout and log

99b16cf

Signed-off-by: karthik balasubramanian <karthikbalasubramanian08@gmail.com>

kaizakin force-pushed the feat/newlogger branch from 20b0774 to 99b16cf Compare March 7, 2026 16:39

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 7, 2026

IrvingMg reviewed Mar 7, 2026

View reviewed changes

openshift-ci bot assigned IrvingMg Mar 7, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 7, 2026

SaschaSchwarze0 requested changes Mar 9, 2026

View reviewed changes

SaschaSchwarze0 added this to the release-v0.20.0 milestone Mar 9, 2026

refactor: rename io to ioStreams across build and bundle commands

a32c596

Signed-off-by: karthik balasubramanian <karthikbalasubramanian08@gmail.com>

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 9, 2026

SaschaSchwarze0 approved these changes Mar 10, 2026

View reviewed changes

openshift-ci bot assigned SaschaSchwarze0 Mar 10, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 10, 2026

openshift-merge-bot bot merged commit 24b225a into shipwright-io:main Mar 10, 2026
10 checks passed

github-project-automation bot moved this to Done in Issues Mar 10, 2026

Conversation

kaizakin commented Feb 23, 2026 • edited by adambkaplan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaizakin commented Mar 3, 2026

Uh oh!

kaizakin commented Mar 3, 2026

Uh oh!

IrvingMg commented Mar 4, 2026

Uh oh!

IrvingMg Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

kaizakin Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

IrvingMg Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

kaizakin Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

IrvingMg left a comment

Choose a reason for hiding this comment

Uh oh!

SaschaSchwarze0 Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

kaizakin Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kaizakin commented Feb 23, 2026 •

edited by adambkaplan

Loading