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

[v6.x backport] src: do not include x.h if x-inl.h is included #16610

Closed

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 30, 2017

Refs: #16518
Refs: #16548

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
Affected core subsystem(s)

src, doc

Make node_crypto_clienthello-inl.h self-contained.

PR-URL: nodejs#16518
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Fixes: nodejs#16519
PR-URL: nodejs#16548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@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. v6.x labels Oct 30, 2017
@joyeecheung
Copy link
Member Author

Skipped the doc changes because they depends on a bunch of other changes on CONTRIBUTING.md that are not backported. I think we don't really need to backport those to v6.x anyway. CI: https://ci.nodejs.org/job/node-test-pull-request/11089/

@joyeecheung
Copy link
Member Author

This should be landed with #16639, will update this PR later.

@MylesBorins
Copy link
Contributor

@joyeecheung please let me know when this is updated

PR-URL: nodejs#16639
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung
Copy link
Member Author

@MylesBorins updated. Also I think I have an idea about #16639 (comment) now..probably need to add a comment there.

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

@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 15, 2017

Looks like the pis are down again..also the checks on this page are showing the build status before the update. Will need to relaunch CI later.

@joyeecheung
Copy link
Member Author

MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Make node_crypto_clienthello-inl.h self-contained.

Backport-PR-URL: #16610
PR-URL: #16518
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Backport-PR-URL: #16610
Fixes: #16519
PR-URL: #16548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Backport-PR-URL: #16610
PR-URL: #16639
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 04ea78d...c8cf488

MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
Make node_crypto_clienthello-inl.h self-contained.

Backport-PR-URL: #16610
PR-URL: #16518
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
Backport-PR-URL: #16610
Fixes: #16519
PR-URL: #16548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
Backport-PR-URL: #16610
PR-URL: #16639
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
Make node_crypto_clienthello-inl.h self-contained.

Backport-PR-URL: #16610
PR-URL: #16518
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
Backport-PR-URL: #16610
Fixes: #16519
PR-URL: #16548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
Backport-PR-URL: #16610
PR-URL: #16639
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
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