8000 output: refactor error handling and fix io.Reader implementation by bobheadxi · Pull Request #32 · sourcegraph/run · GitHub
[go: up one dir, main page]

Skip to content

Conversation

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

Removes all the command-error-waiting stuff with a simple writer.CloseWithError - this means that read operations now return our command error after the buffer is cleared from reads. This greatly simplifies most of the output code - now, we can simply bubble up errors from read operations. It also means the io.Reader implementation is now correct (closes #23)!

Real-life example that now works thanks to the updated io.Reader implementation: https://github.com/sourcegraph/sourcegraph/pull/35836

@bobheadxi bobheadxi requested a review from michaellzc as a code owner May 20, 2022 22:55
@codecov-commenter
Copy link
codecov-commenter commented May 20, 2022

Codecov Report

Merging #32 (5db1f2a) into main (585cd79) will increase coverage by 3.37%.
The diff coverage is 97.95%.

@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   69.68%   73.06%   +3.37%     
==========================================
  Files           8        8              
  Lines         353      297      -56     
==========================================
- Hits          246      217      -29     
+ Misses         86       63      -23     
+ Partials       21       17       -4     
Impacted Files Coverage Δ
command.go 60.00% <ø> (ø)
output.go 86.57% <97.95%> (+9.50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 585cd79...5db1f2a. Read the comment docs.

@bobheadxi bobheadxi force-pushed the implement-io-reader-correctly branch from 9e59e92 to 20716a9 Compare May 21, 2022 01:41
@bobheadxi bobheadxi merged commit ce5203d into main May 21, 2022
@bobheadxi bobheadxi deleted the implement-io-reader-correctly branch May 21, 2022 21:50
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.

io.Reader implementation probably incorrect

4 participants

0