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

lib: make c, ca and certs const in _tls_common #20073

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Apr 16, 2018

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

@nodejs-github-bot nodejs-github-bot added the tls Issues and PRs related to the tls subsystem. label Apr 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Apr 16, 2018

var i;
var val;

if (context) return c;

// NOTE: It's important to add CA before the cert to be able to load
// cert's issuer in C++ code.
var ca = options.ca;
const ca = options.ca;
Copy link
Member

Choose a reason for hiding this comment

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

Use destructuring here and for cert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds good. I'll update. Thx

@danbev
Copy link
Contributor Author

danbev commented Apr 19, 2018

Landed in 188ed07

@danbev danbev closed this Apr 19, 2018
@danbev danbev deleted the _tls_common_consts branch April 19, 2018 03:43
danbev added a commit that referenced this pull request Apr 19, 2018
PR-URL: #20073
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Apr 19, 2018
PR-URL: #20073
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants