Skip to content
New issue

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

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

Already on GitHub? # to your account

[v11.x backport] tls cleanups #25968

Closed

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented Feb 6, 2019

Backport of #25861 and #25929

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++. lib / src Issues and PRs related to general changes in the lib or src directory. v11.x labels Feb 6, 2019
@sam-github sam-github force-pushed the backport-25861-to-v11.x branch from 3a80599 to 7fa600f Compare February 6, 2019 20:11
@sam-github sam-github mentioned this pull request Feb 6, 2019
3 tasks
@addaleax
Copy link
Member

addaleax commented Feb 6, 2019

Looks like the Travis failure is real?

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

- Don't use both break and return simultaneously.
- Use case:/UNREACHABLE() to enforce that all cases are handled, instead
  of CHECK().

Backport-PR-URL: nodejs#25968
PR-URL: nodejs#25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
PR-URL: nodejs#25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-PR-URL: nodejs#25968
@sam-github
Copy link
Contributor Author

I dropped tls: null not valid as a renegotiate callback, its not semver-major in itself, but its a fix to a check that is not being merged down to 11.x because it is semver-major: #25876

The other two can be landed on 11.x

@sam-github sam-github force-pushed the backport-25861-to-v11.x branch from 7fa600f to 29f040a Compare February 7, 2019 22:16
@targos
Copy link
Member

targos commented Feb 8, 2019

@addaleax
Copy link
Member

addaleax commented Feb 8, 2019

@addaleax
Copy link
Member

addaleax commented Feb 8, 2019

Landed in 7cf484c, fb83f84

@addaleax addaleax closed this Feb 8, 2019
addaleax pushed a commit that referenced this pull request Feb 8, 2019
- Don't use both break and return simultaneously.
- Use case:/UNREACHABLE() to enforce that all cases are handled, instead
  of CHECK().

Backport-PR-URL: #25968
PR-URL: #25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
addaleax pushed a commit that referenced this pull request Feb 8, 2019
PR-URL: #25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Backport-PR-URL: #25968

Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@sam-github sam-github deleted the backport-25861-to-v11.x branch February 9, 2019 01:29
sam-github added a commit to sam-github/node that referenced this pull request Apr 29, 2019
- Don't use both break and return simultaneously.
- Use case:/UNREACHABLE() to enforce that all cases are handled, instead
  of CHECK().

Backport-PR-URL: nodejs#25968
PR-URL: nodejs#25861
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
# for free to join this conversation on GitHub. Already have an account? # 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.

5 participants