Skip to content

Conversation

davedoesdev
Copy link
Contributor

See issue comment

This isn't needed for Node 15 but does fix Node 12. I couldn't see a way to make a PR against Node 12 however. The diff for the Node 12 fix is in the issue comment.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. labels Nov 23, 2020
@Trott
Copy link
Member

Trott commented Nov 25, 2020

@nodejs/http2

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 26, 2020
@nodejs-github-bot

This comment has been minimized.

@Trott
Copy link
Member

Trott commented Nov 27, 2020

@nodejs/releasers Could/should this be targeted at the v12.x-staging branch rather than master? Or is targeting the master branch the right way to go?

@mcollina
Copy link
Member

I think we should add the test to master anyway - I'm not sure we should skip the fix.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina
Copy link
Member

This needs another review and it can land cc @nodejs/http2 @ronag @jasnell

Fixes: #33156

PR-URL: #36241
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Trott commented Dec 1, 2020

Landed in 83166fb

@Trott Trott merged commit 83166fb into nodejs:master Dec 1, 2020
danielleadams pushed a commit that referenced this pull request Dec 7, 2020
Fixes: #33156

PR-URL: #36241
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Dec 7, 2020
cjihrig pushed a commit to cjihrig/node that referenced this pull request Dec 8, 2020
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
Fixes: #33156

PR-URL: #36241
Backport-PR-URL: #36372
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
BethGriggs added a commit that referenced this pull request Dec 15, 2020
Notable Changes:

- **deps**:
  - upgrade npm to 6.14.9 (Myles Borins)
    #36450
  - update acorn to v8.0.4 (Michaël Zasso)
    #35791
- **doc**: add release key for Danielle Adams (Danielle Adams)
    #35545
- **http2**: check write not scheduled in scope destructor (David Halls)
    #36241
- **stream**: fix regression on duplex end (Momtchil Momtchev)
    #35941

PR-URL: #36476
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
Fixes: #33156

PR-URL: #36241
Backport-PR-URL: #36372
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs added a commit that referenced this pull request Dec 15, 2020
Notable Changes:

- **deps**:
  - upgrade npm to 6.14.9 (Myles Borins)
    #36450
  - update acorn to v8.0.4 (Michaël Zasso)
    #35791
- **doc**: add release key for Danielle Adams (Danielle Adams)
    #35545
- **http2**: check write not scheduled in scope destructor (David Halls)
    #36241
- **stream**: fix regression on duplex end (Momtchil Momtchev)
    #35941

PR-URL: #36476
BethGriggs added a commit that referenced this pull request Dec 15, 2020
Notable Changes:

- **deps**:
  - upgrade npm to 6.14.9 (Myles Borins)
    #36450
  - update acorn to v8.0.4 (Michaël Zasso)
    #35791
- **doc**: add release key for Danielle Adams (Danielle Adams)
    #35545
- **http2**: check write not scheduled in scope destructor (David Halls)
    #36241
- **stream**: fix regression on duplex end (Momtchil Momtchev)
    #35941

PR-URL: #36476
ruyadorno pushed a commit that referenced this pull request Feb 8, 2021
Fixes: #33156

PR-URL: #36241
Backport-PR-URL: #36355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Feb 9, 2021
ruyadorno pushed a commit that referenced this pull request Feb 10, 2021
Fixes: #33156

PR-URL: #36241
Backport-PR-URL: #36355
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants