Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: multiple http2 refactorings #32551

Closed
wants to merge 3 commits into from
Closed

src: multiple http2 refactorings #32551

wants to merge 3 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Mar 29, 2020

  • Use smart pointers for nghttp2 structs
  • Cleanup for consistent c++ styling
  • BooleanValue => IsTrue/IsFalse
  • Suppress compiler warnings on Windows
  • Simplify Http2Priority struct
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • 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++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Mar 29, 2020
jasnell added 3 commits March 29, 2020 10:42
@jasnell
src: use smart pointers for nghttp2 objects
9c59d2d 
Signed-off-by: James M Snell <[email protected]>
@jasnell
src: rename http2 class and suppress compile warnings
876ae57 
Suppress compile warnings on Windows, rename class for
consistent styling.

Signed-off-by: James M Snell <[email protected]>
@jasnell
src: minor http2 refactorings
9db4418 
* Simplify Http2Priority struct
* BooleanValue => IsTrue/IsFalse

Signed-off-by: James M Snell <[email protected]>
@jasnell jasnell force-pushed the http2-refactorings branch from 3e118d6 to 9db4418 Compare March 29, 2020 17:42
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30178/

addaleax
addaleax approved these changes Mar 29, 2020
View reviewed changes
src/node_http2.cc Show resolved Hide resolved
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 29, 2020 •
edited by addaleax
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/30180/ (:heavy_check_mark:)

@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2020
@himself65 himself65 mentioned this pull request Mar 30, 2020
Closed
2 tasks
Trott
Trott approved these changes Mar 30, 2020
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Semi-rubber-stamp LGTM

addaleax pushed a commit that referenced this pull request Apr 2, 2020
@jasnell @addaleax
src: use smart pointers for nghttp2 objects
8f8bbc6 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this pull request Apr 2, 2020
@jasnell @addaleax
src: rename http2 class and suppress compile warnings
5f5d380 
Suppress compile warnings on Windows, rename class for
consistent styling.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
addaleax pushed a commit that referenced this pull request Apr 2, 2020
@jasnell @addaleax
src: minor http2 refactorings
0be9ebb 
* Simplify Http2Priority struct
* BooleanValue => IsTrue/IsFalse

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@addaleax
Copy link
Member

addaleax commented Apr 2, 2020

Landed in f558a76...0be9ebb

@addaleax addaleax closed this Apr 2, 2020
BethGriggs pushed a commit that referenced this pull request Apr 7, 2020
@jasnell @BethGriggs
src: use smart pointers for nghttp2 objects
f725a5e 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 7, 2020
@jasnell @BethGriggs
src: rename http2 class and suppress compile warnings
0c80086 
Suppress compile warnings on Windows, rename class for
consistent styling.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 7, 2020
@jasnell @BethGriggs
src: minor http2 refactorings
934aa05 
* Simplify Http2Priority struct
* BooleanValue => IsTrue/IsFalse

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request Apr 12, 2020
@jasnell @targos
src: use smart pointers for nghttp2 objects
afc6a25 
Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request Apr 12, 2020
@jasnell @targos
src: rename http2 class and suppress compile warnings
8f766e8 
Suppress compile warnings on Windows, rename class for
consistent styling.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request Apr 12, 2020
@jasnell @targos
src: minor http2 refactorings
62db9a0 
* Simplify Http2Priority struct
* BooleanValue => IsTrue/IsFalse

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32551
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@targos targos mentioned this pull request Apr 13, 2020
Merged
@targos targos added backport-blocked-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 22, 2020
@joyeecheung joyeecheung mentioned this pull request Jun 8, 2020
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@himself65 himself65 himself65 left review comments

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jasnell @nodejs-github-bot @addaleax @Trott @himself65 @targos