-
Notifications
You must be signed in to change notification settings - Fork 592
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
Rename IceSSL namespace to Ice::SSL #2119
Conversation
cpp/src/Ice/Instance.cpp
Outdated
@@ -3,7 +3,7 @@ | |||
// | |||
|
|||
#include "Instance.h" | |||
#include "../IceSSL/SSLEngine.h" | |||
#include "../Ice/SSL/SSLEngine.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should use this include syntax.
If it's a public header, use "Ice/SSL/SSLEngine.h". If it's a private (local) header, use "SSL/SSLEngine.h".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is likely a typo.
|
||
#include "Certificate.h" | ||
|
||
#include <openssl/pem.h> | ||
#include <openssl/x509v3.h> | ||
|
||
namespace IceSSL::OpenSSL | ||
namespace Ice::SSL::OpenSSL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we keep this public API, I would prefer a prefix (OpenSSLCertificate) over a sub-namespace, just like we did for the authentication options.
cpp/src/Ice/Instance.cpp
Outdated
@@ -3,7 +3,7 @@ | |||
// | |||
|
|||
#include "Instance.h" | |||
#include "../IceSSL/SSLEngine.h" | |||
#include "../Ice/SSL/SSLEngine.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this is likely a typo.
@@ -34,10 +34,10 @@ namespace | |||
}; | |||
using CertInfoHolderPtr = shared_ptr<CertInfoHolder>; | |||
|
|||
class SCHannelX509ExtensionI : public X509Extension | |||
class ScHannelX509ExtensionI : public X509Extension |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this casing correct? Shouldn't these be Schannel
?
This PR renames IceSSL namespace to Ice::SSL, It also fixes SSL and Schannel spelling