Skip to content

Conversation

@danbev
Copy link
Contributor

This commit removes the explicit namespace qualifiers in
Connection::Initialize, and SecureContext:Initialize which should not be
necessary in a member function (using an unqualified name should resolve
by searching the class of the member function).

This is consistent with the other intialize functions in node_crypto
like Hmac::Initialize, Hash::Initialize, Sign::Initialize,
Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize,
and InitCrypto.

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

crypto

This commit removes the explicit namespace qualifiers in Connection::Initialize, and SecureContext:Initialize which should not necessary in a member function (using an unqualified name should resolve by searching the class of the member function). This is consistent with the other intialize functions in node_crypto like Hmac::Initialize, Hash::Initialize, Sign::Initialize, Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize, and InitCrypto.
@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 Dec 6, 2017
@danbev
Copy link
ContributorAuthor

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 8, 2017
@apapirovski
Copy link
Contributor

Landed in 987b1e4

apapirovski pushed a commit that referenced this pull request Dec 9, 2017
This commit removes the explicit namespace qualifiers in Connection::Initialize, and SecureContext:Initialize which should not necessary in a member function (using an unqualified name should resolve by searching the class of the member function). This is consistent with the other intialize functions in node_crypto like Hmac::Initialize, Hash::Initialize, Sign::Initialize, Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize, and InitCrypto. PR-URL: #17490 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
This commit removes the explicit namespace qualifiers in Connection::Initialize, and SecureContext:Initialize which should not necessary in a member function (using an unqualified name should resolve by searching the class of the member function). This is consistent with the other intialize functions in node_crypto like Hmac::Initialize, Hash::Initialize, Sign::Initialize, Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize, and InitCrypto. PR-URL: #17490 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
This commit removes the explicit namespace qualifiers in Connection::Initialize, and SecureContext:Initialize which should not necessary in a member function (using an unqualified name should resolve by searching the class of the member function). This is consistent with the other intialize functions in node_crypto like Hmac::Initialize, Hash::Initialize, Sign::Initialize, Verify::Initialize, DiffieHellman::Initialize, ECDH::Initialize, and InitCrypto. PR-URL: #17490 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Dec 12, 2017
@addaleaxaddaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 13, 2017
@gibfahn
Copy link
Member

Should this be backported to v6.x and v8.x staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

@danbev
Copy link
ContributorAuthor

@gibfahn I don't think this is worth backporting and I've added the dont labels.

@danbevdanbev deleted the crypto_consistent_init_functions branch February 28, 2018 07:28
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.

8 participants

@danbev@apapirovski@gibfahn@jasnell@addaleax@cjihrig@BethGriggs@nodejs-github-bot