Skip to content

cli/command/system: add utilities for printing, don't pass CLI if not needed - #4181

Merged
thaJeztah merged 9 commits into
docker:masterfrom
thaJeztah:pass_streams_not_cli
May 3, 2023
Merged

cli/command/system: add utilities for printing, don't pass CLI if not needed#4181
thaJeztah merged 9 commits into
docker:masterfrom
thaJeztah:pass_streams_not_cli

Conversation

@thaJeztah

@thaJeztahthaJeztah commented Apr 10, 2023

Copy link
Copy Markdown
Member

github.com/docker/cli/cli/command/system: add BenchmarkPrettyPrintInfo

goos: linux
goarch: arm64
pkg: github.com/docker/cli/cli/command/system
BenchmarkPrettyPrintInfo
BenchmarkPrettyPrintInfo-5 189028 6156 ns/op 1776 B/op 88 allocs/op

cli/command/system: add utilities for printing, don't pass CLI if not needed

Many of these functions only needed an io.Writer (stdout), so change their
signature to reflect that.

Also adding some utilities to print the output, to keep the linters happier
without having to either suppress errors, or ignore them.

Perhaps we should consider adding utilities for this on the "command.Streams"
outputs.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztahthaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Apr 10, 2023
@codecov-commenter

codecov-commenter commented Apr 10, 2023

Copy link
Copy Markdown

Codecov Report

Merging #4181 (90842dc) into master (afd320c) will increase coverage by 0.00%.
The diff coverage is 86.89%.

Additional details and impacted files
@@ Coverage Diff @@## master #4181 +/- ##
=======================================
Coverage 58.87% 58.88% =======================================
Files 570 570 Lines 49558 49566 +8 =======================================
+ Hits 29178 29186 +8 
Misses 18616 18616 Partials 1764 1764 

@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch 3 times, most recently from 8f15df6 to 84ab1fcCompareApril 13, 2023 12:33
@thaJeztah
thaJeztah marked this pull request as ready for review April 13, 2023 12:33
@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch from 84ab1fc to 7102b83CompareApril 13, 2023 20:33
@thaJeztah
thaJeztah marked this pull request as draft April 13, 2023 22:28
@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch from 7102b83 to 06d1baaCompareApril 13, 2023 22:52
@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch from 06d1baa to 7d4c778CompareApril 26, 2023 11:07
@thaJeztah
thaJeztah marked this pull request as ready for review April 26, 2023 11:08
 goos: linux
goarch: arm64
pkg: github.com/docker/cli/cli/command/system
BenchmarkPrettyPrintInfo
BenchmarkPrettyPrintInfo-5 189028 6156 ns/op 1776 B/op 88 allocs/op
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch from 7d4c778 to 90842dcCompareMay 2, 2023 13:49
…ith import
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't require whole of DockerCLI to be passed, as all we need is a writer.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Don't require whole of DockerCLI to be passed, as all we need is a writer.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No need to pass whole of DockerCLI, as all it needs is the outputs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No need to pass whole of DockerCLI, as all it needs is the outputs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
No need to pass whole of DockerCLI, as all it needs is the outputs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adding some utilities to print the output, to keep the linters happier
without having to either suppress errors, or ignore them.
Perhaps we should consider adding utilities for this on the "command.Streams"
outputs.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the pass_streams_not_cli branch from 90842dc to 155f7d9CompareMay 2, 2023 14:56

@sam-thibaultsam-thibault 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.

LGTM

@thaJeztah
thaJeztah merged commit 054343b into docker:masterMay 3, 2023
@thaJeztah
thaJeztah deleted the pass_streams_not_cli branch May 3, 2023 14:43
@thaJeztahthaJeztah added this to the 24.0.0 milestone May 3, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactorPR's that refactor, or clean-up codestatus/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah@codecov-commenter@sam-thibault