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.

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

http2: rename http2_state class to Http2State #20423

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 30, 2018

This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

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

This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.
@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 Apr 30, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 30, 2018

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

SGTM, but maybe we should note in the style guide that for C-like plain structs we tend to follow snake_case style, e.g. struct http2_state_internal?

@danbev
Copy link
Contributor Author

danbev commented May 1, 2018

SGTM, but maybe we should note in the style guide that for C-like plain structs we tend to follow snake_case style, e.g. struct http2_state_internal?

Sounds good, I'll add a suggestion for this. Thanks.

@danbev
Copy link
Contributor Author

danbev commented May 2, 2018

node-test-commit-arm-fanned failure looks unrelated

console output:

09:53:55 not ok 303 sequential/test-net-GH-5504
09:53:55   ---
09:53:55   duration_ms: 240.39
09:53:55   severity: fail
09:53:55   exitcode: -15
09:53:55   stack: |-
09:53:55     timeout
09:53:55     SERVER 1>listening
09:53:55     SERVER 2>NET 24180: setupListenHandle 127.0.0.1 12346 4 false undefined
09:53:55     SERVER 2>NET 24180: setupListenHandle: create a handle
09:53:55     SERVER 2>NET 24180: bind to 127.0.0.1
09:53:55     CLIENT 2>NET 24186: createConnection [ { host: '127.0.0.1', port: 12346 },
09:53:55     CLIENT 2>  [Function],
09:53:55     CLIENT 2>  [Symbol(normalizedArgs)]: true ]
09:53:55     CLIENT 2>NET 24186: pipe false undefined
09:53:55     SERVER 2>NET 24180: onconnection
09:53:55     CLIENT 2>NET 24186: afterConnect
09:53:55     SERVER 2>NET 24180: _read
09:53:55     SERVER 2>NET 24180: Socket._read readStart
09:53:55     SERVER 2>NET 24180: SERVER _emitCloseIfDrained
09:53:55     SERVER 2>NET 24180: SERVER handle? false   connections? 1
09:53:55     CLIENT 2>NET 24186: destroy
09:53:55     CLIENT 2>NET 24186: close
09:53:55     CLIENT 2>NET 24186: close handle
09:53:55     CLIENT 2>NET 24186: _read
09:53:55     CLIENT 2>NET 24186: _read wait for connection
09:53:55     CLIENT 2>NET 24186: emit close
09:53:55   ...
node-test-linux-linked-openssl110 failure looks unrelated

console output:

09:52:55 not ok 174 parallel/test-child-process-spawnsync-kill-signal
09:52:55   ---
09:52:55   duration_ms: 0.691
09:52:55   severity: fail
09:52:55   exitcode: 1
09:52:55   stack: |-
09:52:55     assert.js:77
09:52:55       throw new AssertionError(obj);
09:52:55       ^
09:52:55     
09:52:55     AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
09:52:55     + expected - actual
09:52:55     
09:52:55     - 'SIGSEGV'
09:52:55     + 'SIGTERM'
09:52:55         at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/nodes/ubuntu1604_sharedlibs_openssl110_x64/test/parallel/test-child-process-spawnsync-kill-signal.js:42:12)
09:52:55         at Module._compile (internal/modules/cjs/loader.js:678:30)
09:52:55         at Object.Module._extensions..js (internal/modules/cjs/loader.js:689:10)
09:52:55         at Module.load (internal/modules/cjs/loader.js:589:32)
09:52:55         at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
09:52:55         at Function.Module._load (internal/modules/cjs/loader.js:520:3)
09:52:55         at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)
09:52:55         at startup (internal/bootstrap/node.js:229:19)
09:52:55         at bootstrapNodeJSCore (internal/bootstrap/node.js:577:3)
09:52:55   ...

This commit adds a section mentioning that for C-like structs it is
alright to use snake_case.

Refs: nodejs#20423
@addaleax
Copy link
Member

addaleax commented May 6, 2018

Thanks! Landed in 2c5b94f, c8f8847

@addaleax addaleax closed this May 6, 2018
addaleax pushed a commit that referenced this pull request May 6, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request May 6, 2018
This commit adds a section mentioning that for C-like structs it is
alright to use snake_case.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 8, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 8, 2018
This commit adds a section mentioning that for C-like structs it is
alright to use snake_case.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 8, 2018
MylesBorins pushed a commit that referenced this pull request May 8, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 8, 2018
This commit adds a section mentioning that for C-like structs it is
alright to use snake_case.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 9, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 9, 2018
This commit adds a section mentioning that for C-like structs it is
alright to use snake_case.

PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
kjin pushed a commit to kjin/node that referenced this pull request Aug 23, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: nodejs#20423
Reviewed-By: Anna Henningsen <[email protected]>
kjin pushed a commit to kjin/node that referenced this pull request Sep 11, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: nodejs#20423
Reviewed-By: Anna Henningsen <[email protected]>
kjin pushed a commit to kjin/node that referenced this pull request Sep 19, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: nodejs#20423
Reviewed-By: Anna Henningsen <[email protected]>
kjin pushed a commit to kjin/node that referenced this pull request Oct 16, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

PR-URL: nodejs#20423
Reviewed-By: Anna Henningsen <[email protected]>
BethGriggs pushed a commit that referenced this pull request Oct 17, 2018
This commit renames the http2_state class to follow the guidelines in
CPP_STYLE_GUIDE.md.

Backport-PR-URL: #22850
PR-URL: #20423
Reviewed-By: Anna Henningsen <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Oct 30, 2018
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++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants