8000 output: use io.Reader implementation more internally by bobheadxi · Pull Request #35 · sourcegraph/run · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@bobheadxi
Copy link
Member
@bobheadxi bobheadxi commented May 27, 2022

Replace internal .Stream usage with approaches that use io more instead, which was fixed in #32 . tl;dr more idiomatic Go code

@bobheadxi bobheadxi requested a review from a team May 27, 2022 17:00
@bobheadxi bobheadxi requested a review from michaellzc as a code owner May 27, 2022 17:00
Copy link
Contributor
@jhchabran jhchabran left a comment

Choose a reason for hiding this comment

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

Nice ☺️

if err != nil {
return "", err
}
return strings.TrimSuffix(string(b), "\n"), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't strings.TrimSpace what we're looking for here?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are specifically trimming only the tailing new line that we added as we write output back line-by-line - we don't want to trim indentation, for example

@bobheadxi bobheadxi merged commit 40af693 into main May 30, 2022
@bobheadxi bobheadxi deleted the internal-io-reader-use branch May 30, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0