Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
DOCSP-45179 Configure TLS #129
DOCSP-45179 Configure TLS #129
Changes from 3 commits
ede66c1
53eff61
aa0e740
749241a
673a588
3eec033
3693caa
0246516
610f989
65c0306
f4dda22
3867434
083e7f5
b64d746
6a574d8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
s: instead of spelling this out, you could put the options in a table that includes a column for 'data type'
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.
s: shortened
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.
Thanks for the edit! Will keep in the part about still needing to specify both options, as I think it's important to clarify.
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.
q: is 'identify the connection against mongodb' the right language? i would have gone for 'identify the client to MongoDB' or something similar.
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.
'identify the connection against mongodb' is the language used in the API documentation, but it is a bit strange sounding. Changed to 'verify the connection to MongoDB' .
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.
s: does this mean server to client, client to server, or both? i would be specific.
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.
Changed to say 'from the Mongo deployment to the client', which I think is the correct flow. I can ask the technical reviewer to make sure.
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.
q: this section talks only about TLS options. is 'Modify the TLS Context' a more appropriate heading?
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.
Since the method has TLS in it, sounds good! I edited the following text, since Ruby calls it SSL Context.
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.
For technical reviewer, is this an appropriate link to put for reference? I wasn't able to find much information on
context
options in our docs. Thanks!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.
Ruby 2.7 is EOL, so that link is probably not ideal. Linking out to the Ruby docs is otherwise probably fine, we just need to make sure the links remain relevant.
Here's the link to the same class, for Ruby 3.2.6: https://ruby-doc.org/3.2.6/exts/openssl/OpenSSL/SSL/SSLContext.html