Skip to content
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 29902 revise quick reference examples #277

Merged

Conversation

nickldp
Copy link
Contributor

@nickldp nickldp commented Jul 7, 2023

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-29902
Staging

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?

Copy link
Collaborator

@rustagir rustagir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work so far! i think some of the examples should be changed to be more intuitive. if you want to use abstract examples, try to make them as basic as possible and have the fields be x, y, z, or something like that, instead of matching points to fields. users could get confused why point DEF has an xValue for example.

@@ -28,13 +28,13 @@ their related reference and API documentation.
.. input::
:language: go

err = coll.FindOne(context.TODO(), bson.D{{"rating", 5}}).Decode(&result)
err = coll.FindOne(context.TODO(), bson.D{{"xValue", 5}}).Decode(&result)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: I think that this example is still a little difficult to follow. I noticed that elsewhere on this page you used sample documents that includes names -- maybe you could also use that here with an age filter or something!

Suggestion applies to all examples that use this sample data

@@ -48,14 +48,14 @@ their related reference and API documentation.
.. input::
:language: go

cursor, err := coll.Find(context.TODO(), bson.D{{"rating", bson.D{{"$gte", 8}}}})
cursor, err := coll.Find(context.TODO(), bson.D{{"point", bson.D{{"$gte", 8}}}})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I: In this example, the query filter doesn't match the output. The query searches for point values gte 8, but the point field isn't numerical. In any case, see comment above for changing example

@@ -434,16 +434,16 @@ their related reference and API documentation.
context.TODO(),
bson.D{},
options.Find().SetProjection(
bson.D{{"vendor", 0}, {"_id",0}}
bson.D{{"point", 0}, {"_id",0}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I: the projection sets point to 0 but the output still includes the field
S: omit the unprojected field from the output

@@ -469,10 +469,10 @@ their related reference and API documentation.
:language: go

// only searches fields with text indexes
cursor, err := coll.Find(context.TODO(), bson.D{{"$text", bson.D{{"$search", "Breakfast"}}}})
cursor, err := coll.Find(context.TODO(), bson.D{{"$text", bson.D{{"$search", "Beagle"}}}})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: use an example where the search term is only part of a phrase
for example, the search term could be sports and the matched document could be
`{ "name": "Sam", "description": "I love playing sports. My favorite sports are volleyball and water polo."}

@nickldp nickldp requested a review from rustagir July 7, 2023 18:04
Copy link
Collaborator

@rustagir rustagir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! good work responding to my comments.

Comment on lines 134 to 135
bson.D{{"age", bson.D{{"$lt", 10}}}},
bson.D{{"$set", bson.D{{"age", 1}}}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: there's nothing wrong with this per say, but it might be confusing for readers to see an example where youre setting the age to 1 just because its less than 10. no need to change, but just suggesting

)
)

.. output::
:language: go
:visible: false

[{type Masala} {rating 10}]
[{type English Breakfast} {rating 8}]
[{firstName Lester} ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[{firstName Lester} ]
[{firstName Lester}]

Comment on lines 477 to 478
[{"firstName": "Emily" , "Description": "I love to play sports and
walk my beagle."} ... ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: make one line


.. output::
:language: go
:visible: false

[{type English Breakfast} {rating 8} ... ]
[{"firstName": "Emily" , "Description": "I love to play sports and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[{"firstName": "Emily" , "Description": "I love to play sports and
[{"firstName": "Emily" , "description": "I love to play sports and

@nickldp nickldp merged commit f7d1a93 into mongodb:master Jul 7, 2023
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
rustagir pushed a commit that referenced this pull request Jul 14, 2023
* updated examples

* revised

* fix

* post review

* final

(cherry picked from commit f7d1a93)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants