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
Document how to use custom CA in README (#1229) #1236
Conversation
request.get({ | ||
url: 'https://api.some-server.com/', | ||
agentOptions: { | ||
'ca': fs.readFileSync("ca.cert.pem") |
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.
Just a really minor nitpick, but mind changing 'ca'
to just ca
? No apostrophes needed here.
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 am following the convention laid out in previous examples.
From the first SSL/TLS example:
var options = {
url: 'https://api.some-server.com/',
agentOptions: {
'cert': fs.readFileSync(certFile),
'key': fs.readFileSync(keyFile),
// Or use `pfx` property replacing `cert` and `key` when using private key, certificate and CA certs in PFX or PKCS12 format:
// 'pfx': fs.readFileSync(pfxFilePath),
'passphrase': 'password',
'securityOptions': 'SSL_OP_NO_SSLv3'
}
};
This could definitely be corrected, but I don't think a change of style has anything to do with this pull request :)
lgtm, besides that minor nitpick! |
I'm in favor of going ahead and removing the quotes in the other example too, but that's not a big deal to me. More importantly, what does a custom CA actually do? I think users will want to see something like this:
Awkward wording, definitely could be done better. Also, does specifying a custom CA override Node's built-in CAs? I think that's worth clarifying too. |
c11717e
to
512dcfc
Compare
Removed single quotes around field names
I have tried to accomodate the requested changes (which are all very valid). I dont know if the text is the best it gets, but the use case is now clearer.
|
Looks good to me, thanks @hypesystem! |
Document how to use custom CA in README (#1229)
Fixes #1229