Add command.Addwriter() support #1198
Conversation
|
@saschagrunert: Adding label: Reasons for blocking this PR:[Changes to certain release tools can affect our ability to test, build, and release Kubernetes. This PR must be explicitly approved by SIG Release repo admins.] Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Thanks @saschagrunert! One comment but definitely not blocking :)
/lgtm
/hold for other reviewers
| @@ -190,8 +199,12 @@ func (c *Command) run(printOutput bool) (res *Status, err error) { | |||
|
|
|||
| var stdOutWriter, stdErrWriter io.Writer | |||
| if printOutput { | |||
| stdOutWriter = io.MultiWriter(os.Stdout, stdOutBuffer) | |||
| stdErrWriter = io.MultiWriter(os.Stderr, stdErrBuffer) | |||
| stdOutWriter = io.MultiWriter(append( | |||
There was a problem hiding this comment.
Not blocking, but just wondering if you think we would ever want to support supplying separate writers for Stdout and Stderr? I could be mistaken here but it looks like if you want one to be written to a passed in writer, than you must allow for both.
There was a problem hiding this comment.
Yes this gives us more flexibility. I changed the API to allow writers for stdout and stderr separately. Or we just use AddWriter for both.
Updated the test cases as well 👍
This allows commands to specify additional writers, for example when having the need for logging to files. Signed-off-by: Sascha Grunert <sgrunert@suse.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hasheddan, justaugustus, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This allows commands to specify additional writers, for example when
having the need for logging to files.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?