Skip to content

http2: refactor settings handling - #16668

Closed
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:http2-settings-refactor
Closed

http2: refactor settings handling#16668
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:http2-settings-refactor

Conversation

@jasnell

Copy link
Copy Markdown
Member

Add Http2Seettings utility class for handling settings logic and reducing code duplication.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

http2

@nodejs-github-botnodejs-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 Nov 1, 2017

@sebdeckerssebdeckers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is above my pay grade, but LGTM. 😜

@mscdexmscdex added http2 Issues or PRs related to the http2 subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Nov 1, 2017

@apapirovskiapapirovski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@XadillaXXadillaX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@jasnell

Copy link
Copy Markdown
MemberAuthor

@jasnell
jasnellforce-pushed the http2-settings-refactor branch from 8a644c8 to 57a18d2CompareNovember 2, 2017 06:03

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Add `Http2Seettings` utility class for handling settings
logic and reducing code duplication.
@jasnell
jasnellforce-pushed the http2-settings-refactor branch from 57a18d2 to 18b3123CompareNovember 3, 2017 17:14
jasnell added a commit that referenced this pull request Nov 3, 2017
Add `Http2Seettings` utility class for handling settings
logic and reducing code duplication.
PR-URL: #16668
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@jasnell

Copy link
Copy Markdown
MemberAuthor

Landed in de24602

@jasnelljasnell closed this Nov 3, 2017
cjihrig pushed a commit to cjihrig/node that referenced this pull request Nov 6, 2017
Add `Http2Seettings` utility class for handling settings
logic and reducing code duplication.
PR-URL: nodejs#16668
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@cjihrigcjihrig mentioned this pull request Nov 6, 2017
gibfahn pushed a commit that referenced this pull request Nov 14, 2017
Add `Http2Seettings` utility class for handling settings
logic and reducing code duplication.
PR-URL: #16668
Reviewed-By: Sebastiaan Deckers <sebdeckers83@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@gibfahngibfahn mentioned this pull request Nov 21, 2017
Sign up for freeto 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++.http2Issues or PRs related to the http2 subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@jasnell@apapirovski@mcollina@sebdeckers@XadillaX@mscdex@MylesBorins@gibfahn@nodejs-github-bot