-
Notifications
You must be signed in to change notification settings - Fork 17
DOCSP-39689 Network Compression standardization #70
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
DOCSP-39689 Network Compression standardization #70
Conversation
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.
LGTM + a couple small things!
Pinging @vbabanin in case this PR request got lost in the mix |
@@ -0,0 +1,19 @@ | |||
// start-specify-connection-string | |||
|
|||
import java.util.Arrays; |
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.
We specify the inclusion of Arrays
as an import here, but it is never used in this example tab. It seems to be related to the next tab, where the compressorList()
method is used.
However, even if we move the import of Arrays
to the next tab, why do we only include this import and omit the imports like MongoClient
and MongoClientSettings
?
The previous page, 'TLS/SSL', includes a statement before examples that lists these import statements:
import com.mongodb.reactivestreams.client.MongoClients;
import com.mongodb.reactivestreams.client.MongoClient;
Perhaps it is assumed that users have read the previous page, 'TLS/SSL' and are aware of these imports. However, I don't think we should make this assumption.
I noticed in another PR that we followed the approach of duplicating the same information to ensure users have everything they need within the section they’re reading, regardless of prior sections. This is highlighted in the discussion here: #48 (comment).
Therefore, it seems that we should add not only the Arrays
import but also MongoClients
and MongoClientSettings
imports to ensure completeness and consistency.
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 catching this! That import was added by mistake (I think my editor automatically added it without me noticing for the following code snippet in that same file). For these pages that focus on a specific topic and don't provide "fully runnable code examples," we typically don't show the imports, as these code examples are intended to show only the relevant lines of code for the operation itself (See the pages nested under "Write Data to MongoDB" for more examples of this). The TLS/SSL page has not yet been standardized so it doesn't follow this convention yet.
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.
Understood, thank you for clarification!
✅ Deploy Preview for docs-java-rs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-39689
Staging - https://preview-mongodbjordansmith721.gatsbyjs.io/java-rs/DOCSP-39689-network-compression/connect-to-mongo/network-compression/
Self-Review Checklist