Skip to content

Conversation

@jscissr
Copy link
Contributor

@jscissr jscissr commented Aug 29, 2017

After these commits, all tests pass in latest Node.js v4, v6 and v8; on Windows and Linux. All changes should be backward compatible.

This contains the changes in molnarg/node-http2#251, as well as some other fixes for things that are only in this fork.

db09362 tried to cleanup old streams, but it deletes them too early. I fixed this by deleting after a timeout, but this timeout probably should be made configurable.

@hthetiot hthetiot self-requested a review August 30, 2017 23:00
@hthetiot hthetiot added the bug label Aug 30, 2017
@hthetiot hthetiot added this to the v4.0.x milestone Aug 30, 2017
@hthetiot hthetiot requested review from dpwspoon and jitsni and removed request for hthetiot August 30, 2017 23:08
@hthetiot hthetiot self-assigned this Aug 30, 2017
if (!stream._ended) {
nextBucket.push(stream);
} else {
delete this._streamIds[stream.id];
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose _removeStream() delete the item in _streamIds

@jitsni
Copy link
Contributor

jitsni commented Aug 31, 2017

looks good to me.

@hthetiot hthetiot merged commit fc9b298 into kaazing:master Aug 31, 2017
@hthetiot hthetiot mentioned this pull request Aug 31, 2017
@jscissr jscissr deleted the fix-tests branch August 31, 2017 22:45
@hthetiot
Copy link
Contributor

hthetiot commented Sep 8, 2017

Thx @jselit it's on master and I will let you know once released on npm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants