Skip to content

refactor: Use IOStreams for command output instead of os.Stdout and log #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaizakin wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −15
Open

refactor: Use IOStreams for command output instead of os.Stdout and log #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions pkg/shp/cmd/build/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package build // nolint:revive

import (
"fmt"
"log"
"os"
"path"
"time"
Expand Down Expand Up @@ -38,8 +37,9 @@ type UploadCommand struct {

sourceBundleImage string // image to be used as the source bundle

pw *reactor.PodWatcher // pod-watcher instance
follower *follower.Follower // follower instance
ioStreams *genericclioptions.IOStreams // io streams for user-facing output
pw *reactor.PodWatcher // pod-watcher instance
follower *follower.Follower // follower instance
}

const (
Expand Down Expand Up @@ -186,7 +186,7 @@ func (u *UploadCommand) createBuildRun(p *params.Params) (*buildv1beta1.BuildRun
flags.SanitizeBuildRunSpec(&br.Spec)

ns := p.Namespace()
log.Printf("Creating a BuildRun for '%s/%s' Build...", ns, u.buildRefName)
fmt.Fprintf(u.ioStreams.Out, "Creating a BuildRun for '%s/%s' Build...\n", ns, u.buildRefName)
clientset, err := p.ShipwrightClientSet()
if err != nil {
return nil, err
Expand All @@ -197,7 +197,7 @@ func (u *UploadCommand) createBuildRun(p *params.Params) (*buildv1beta1.BuildRun
if err != nil {
return nil, err
}
log.Printf("BuildRun '%s' created!", br.GetName())
fmt.Fprintf(u.ioStreams.Out, "BuildRun '%s' created!\n", br.GetName())
return br, nil
}

Expand All @@ -207,7 +207,7 @@ func (u *UploadCommand) performDataStreaming(target *streamer.Target) error {
return nil
}

fmt.Fprintf(os.Stdout, "Streaming %q to the Build POD %q ...\n", u.sourceDir, target.Pod)
fmt.Fprintf(u.ioStreams.Out, "Streaming %q to the Build POD %q ...\n", u.sourceDir, target.Pod)
// creates an in-memory tarball with source directory data, and ready to start data streaming
tarball, err := streamer.NewTar(u.sourceDir)
if err != nil {
Expand Down Expand Up @@ -280,6 +280,7 @@ func (u *UploadCommand) onPodModifiedEventBundling(pod *corev1.Pod) error {
// 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
Copy link
Contributor

@IrvingMg IrvingMg Mar 4, 2026

Choose a reason for hiding this comment

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

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

Copy link
Author

@kaizakin kaizakin Mar 5, 2026

Choose a reason for hiding this comment

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

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

// creating a BuildRun with settings for the local source upload
br, err := u.createBuildRun(p)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions pkg/shp/cmd/buildrun/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package buildrun

import (
"fmt"
"os"
"text/tabwriter"
"time"

Expand Down Expand Up @@ -61,7 +60,7 @@ func (c *ListCommand) Run(params *params.Params, io *genericclioptions.IOStreams
// TODO: Support multiple output formats here, not only tabwriter
// 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)
columnNames := "NAME\tSTATUS\tAGE"
columnTemplate := "%s\t%s\t%s\n"

Expand Down
3 changes: 1 addition & 2 deletions pkg/shp/cmd/buildstrategy/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package buildstrategy

import (
"fmt"
"os"
"text/tabwriter"
"time"

Expand Down Expand Up @@ -50,7 +49,7 @@ func (c *ListCommand) Validate() error {

// Run executes list sub-command logic
func (c *ListCommand) Run(p *params.Params, io *genericclioptions.IOStreams) error {
w := tabwriter.NewWriter(os.Stdout, 0, 8, 2, '\t', 0)
w := tabwriter.NewWriter(io.Out, 0, 8, 2, '\t', 0)
if !c.noHeader {
fmt.Fprintln(w, "NAME\tAGE")
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/shp/cmd/clusterbuildstrategy/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterbuildstrategy

import (
"fmt"
"os"
"text/tabwriter"
"time"

Expand Down Expand Up @@ -49,7 +48,7 @@ func (c *ListCommand) Validate() error {

// Run executes list sub-command logic
func (c *ListCommand) Run(p *params.Params, io *genericclioptions.IOStreams) error {
w := tabwriter.NewWriter(os.Stdout, 0, 8, 2, '\t', 0)
w := tabwriter.NewWriter(io.Out, 0, 8, 2, '\t', 0)
if !c.noHeader {
fmt.Fprintln(w, "NAME\tAGE")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/shp/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var rootCmd = &cobra.Command{
func NewCmdSHP(ioStreams *genericclioptions.IOStreams) *cobra.Command {
p := params.NewParams()
p.AddFlags(rootCmd.PersistentFlags())
rootCmd.AddCommand(version.Command())
rootCmd.AddCommand(version.Command(ioStreams))
rootCmd.AddCommand(build.Command(p, ioStreams))
rootCmd.AddCommand(buildrun.Command(p, ioStreams))
rootCmd.AddCommand(buildstrategy.Command(p, ioStreams))
Expand Down
5 changes: 3 additions & 2 deletions pkg/shp/cmd/version/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
"fmt"

"github.com/spf13/cobra"
"k8s.io/cli-runtime/pkg/genericclioptions"
)

var version string

// Command returns Version subcommand of Shipwright CLI
// for retrieving the shp version
func Command() *cobra.Command {
func Command(ioStreams *genericclioptions.IOStreams) *cobra.Command {
command := &cobra.Command{
Use: "version",
Aliases: []string{"v"},
Expand All @@ -23,7 +24,7 @@ func Command() *cobra.Command {
version = "development"
}

fmt.Printf("version: %s\n", version)
fmt.Fprintf(ioStreams.Out, "version: %s\n", version)
},
}
return command
Expand Down