refactor: pass an io.Writer to image.List#1887
Conversation
| } | ||
|
|
||
| func printImages(ctx context.Context, options types.ImageListCommandOptions, client *containerd.Client, imageList []images.Image) error { | ||
| func printImages(ctx context.Context, imageList []images.Image, options types.ImageListCommandOptions, client *containerd.Client, w io.Writer) error { |
There was a problem hiding this comment.
nit: can you move client to after ctx and before imageList? It's more consistent with other funcs where a client is used. (e.g., (ctx, client, container/image/etc, option, io, etc))
There was a problem hiding this comment.
makes sense, updated.
davidhsingyuchen
force-pushed
the
refactor-image-stdout
branch
from
5604a0c to
fba523d
Compare
| ) | ||
|
|
||
| func List(ctx context.Context, options types.ImageListCommandOptions) error { | ||
| func List(ctx context.Context, options types.ImageListCommandOptions, stdout io.Writer) error { |
There was a problem hiding this comment.
| func List(ctx context.Context, options types.ImageListCommandOptions, stdout io.Writer) error { | |
| func List(ctx context.Context, stdout io.Writer, options types.ImageListCommandOptions) error { |
There was a problem hiding this comment.
@AkihiroSuda Shall we somehow (i.e., open an issue?) standardize the parameter order?
nerdctl/pkg/cmd/image/inspect.go
Line 35 in 35bb03e
- refactor: consolidate main logic of volume.List into volume.Volumes #1837 (comment)
Thanks!
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks, fixed. Shall I also update printImages from
func printImages(ctx context.Context, client *containerd.Client, imageList []images.Image, options types.ImageListCommandOptions, w io.Writer) error {to
func printImages(ctx context.Context, w io.Writer, client *containerd.Client, imageList []images.Image, options types.ImageListCommandOptions) error {There was a problem hiding this comment.
Updated according to the summary in #1881 (comment), thanks!
davidhsingyuchen
force-pushed
the
refactor-image-stdout
branch
from
fba523d to
4ba8b00
Compare
davidhsingyuchen
force-pushed
the
refactor-image-stdout
branch
4 times, most recently
from
fa99db0 to
d4fe593
Compare
davidhsingyuchen
requested review from
AkihiroSuda and
djdongjin
and removed request for
AkihiroSuda
| switch options.Format { | ||
| case "", "table", "wide": | ||
| w = tabwriter.NewWriter(w, 4, 8, 4, ' ', 0) | ||
| options.Stdout = tabwriter.NewWriter(options.Stdout, 4, 8, 4, ' ', 0) |
There was a problem hiding this comment.
nit: I feel we should keep and update the w variable (e.g., line 186-187) and use w here&below, instead of persisting the change in options.Stdout.
There was a problem hiding this comment.
Sure, fixed.
davidhsingyuchen
force-pushed
the
refactor-image-stdout
branch
2 times, most recently
from
0bab337 to
c73b0ba
Compare
Signed-off-by: Hsing-Yu (David) Chen <davidhsingyuchen@gmail.com>
davidhsingyuchen
force-pushed
the
refactor-image-stdout
branch
from
c73b0ba to
9470efa
Compare
3 participants
Part of #1856
Also rearrange the parameter order of
printImages(i.e., moveimageListto be right afterctx) according to the reason stated in #1837 (comment).Signed-off-by: Hsing-Yu (David) Chen davidhsingyuchen@gmail.com