-
Notifications
You must be signed in to change notification settings - Fork 34
DOCSP-51818 Standardize replace usage ex #541
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
base: comp-cov
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-golang ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔄 Deploy Preview for docs-golang processing
|
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 tiny nit and a question 😄
fmt.Println("Number of documents replaced:", result.ModifiedCount) | ||
} | ||
|
||
// When you run this file for the first time, it should print: |
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] What do you think about removing vs keeping these comments at the end of the files? I think I've been removing since the output is shown in the code block but maybe we could align and make sure they're all consistent. I'm fine either way!
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 this particular comment, I think it's good to keep in! If you run this code example twice, it outputs nothing the second time because the document that is being queried for no longer exists.
I also think it's nice to keep because readers will hopefully copy this into their own code editors, and they can have the expected result without referencing the docs again. I am in favor of keeping them in if this makes sense to you!
coll := client.Database("sample_restaurants").Collection("restaurants") | ||
filter := bson.D{{"name", "Madame Vo"}} | ||
filter := RestaurantNameFilter{Name: "Rizzo's Fine Pizza"} |
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.
@lindseymoore Could you clarify the goal of this update? Is it to illustrate how users could use either a bson.D{} or Go struct as the filter argument for driver operations?
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.
Hi, exactly! We received this ticket from the Go PM to add both bson.D and structs to the usage examples : https://jira.mongodb.org/browse/DOCSP-51595 @prestonvasquez
@@ -23,7 +21,9 @@ type Restaurant struct { | |||
Grades []interface{} `bson:"grades,omitempty"` | |||
} | |||
|
|||
// end-restaurant-struct | |||
type RestaurantNameFilter struct { |
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.
Using a struct as a filter is quite awkward and error prone. For example, say you have the following filter struct:
type RNFilter struct {
Name string
Rating int
}
In Go, if you don't populate both fields then the filter will use the "zero" value for Rating:
f := RNFilter{Name: "Rizzo's Fine Pizza"} // Rating is implicitly 0
So if the actual rating is anything other than "0", nothing will be found since the underlying command is something of this shape:
{"name": "Rizzo's Fine Pizza", "rating": 0}
Suggest retaining bson.D
for the filter. Using a go struct for decoding results into is best practice.
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 see, let me check in with @rishitb-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.
I think @prestonvasquez 's thought makes sense. Let's standardize on defining the queries via bson.D
(i.e for any filters) and using the struct to represent the documents (when inserting new documents or retrieving results).
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.
Ok, will do.
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-51818
Staging Links
https://deploy-preview-541--docs-golang.netlify.app/crud/update/replace/#replace-one-document-example--full-file
Self-Review Checklist