Skip to content

Conversation

@addaleax
Copy link
Member

Fixes: #13176

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

src/crypto

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels May 24, 2017


// Currently, EnableTicketKeyCallback and TicketKeyCallback are only present for
// the regression test in test/parallel/test-https-resume-after-renew.js.
Copy link
Contributor

Choose a reason for hiding this comment

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

[suggestion] If i understand this case correctly add , they are considered private until decided otherwise.
Also maybe add _ to the JS binding.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I think it’s implied that they are private when they are only there for tests. Also, this is only on the _handle property, so I think we’re good.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack

@addaleax
Copy link
MemberAuthor

Landed in e2f2d18

@addaleaxaddaleax deleted the ticketkeycallback-comment branch May 26, 2017 15:59
addaleax added a commit that referenced this pull request May 26, 2017
Fixes: #13176 PR-URL: #13193 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
jasnell pushed a commit that referenced this pull request May 28, 2017
Fixes: #13176 PR-URL: #13193 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@jasnelljasnell mentioned this pull request May 28, 2017
@gibfahngibfahn mentioned this pull request Jun 15, 2017
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
Fixes: #13176 PR-URL: #13193 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 18, 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++.cryptoIssues and PRs related to the crypto subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

What does SecureContext::EnableTicketKeyCallback do?

6 participants

@addaleax@refack@lpinca@cjihrig@MylesBorins@nodejs-github-bot