Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

[WIP] fix: propagate trailer errors correctly #636

Merged
merged 2 commits into from
Nov 24, 2017

Conversation

dryajov
Copy link
Contributor

@dryajov dryajov commented Nov 23, 2017

fixes - #607

needs tests

@ghost ghost assigned dryajov Nov 23, 2017
@ghost ghost added the in progress label Nov 23, 2017
@dryajov dryajov changed the title fix: propagate trailer errors correctly [WIP] fix: propagate trailer errors correctly Nov 23, 2017
Copy link
Contributor

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

@dryajov can you add a test?

@pgte mind reviewing this one?

@daviddias daviddias requested a review from pgte November 24, 2017 10:00
Copy link
Contributor

@pgte pgte left a comment

Choose a reason for hiding this comment

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

other than needing tests, LGTM.

@daviddias
Copy link
Contributor

@richardschneider Windows tests seem to be inconsistent (failing again), mind taking a look? It is not due to this PR but since Appveyor was gracious enough to let this PR do a run, we should take advantage and look at what it says.

@dryajov
Copy link
Contributor Author

dryajov commented Nov 24, 2017

@daviddias, yep working on tests right now.

@richardschneider
Copy link
Contributor

It looks like the .refs test is failing because of CRLFs; see ipfs/js-ipfs-repo#153 (comment).

@codecov
Copy link

codecov bot commented Nov 24, 2017

Codecov Report

Merging #636 into master will increase coverage by 0.15%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #636      +/-   ##
==========================================
+ Coverage    84.1%   84.25%   +0.15%     
==========================================
  Files         115      115              
  Lines        1749     1747       -2     
==========================================
+ Hits         1471     1472       +1     
+ Misses        278      275       -3
Impacted Files Coverage Δ
src/utils/send-request.js 89.13% <0%> (+2.81%) ⬆️
src/utils/send-files-stream.js 88.75% <100%> (+0.14%) ⬆️

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 447f362...3863903. Read the comment docs.

@daviddias daviddias merged commit 62d733e into master Nov 24, 2017
@daviddias daviddias deleted the fix/propage-trailer-errors branch November 24, 2017 22:32
@ghost ghost removed the in progress label Nov 24, 2017
@richardschneider
Copy link
Contributor

@diasdavid I'm still getting .refs test failure on windows. Do you want to open a new issue/pr?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants