-
Notifications
You must be signed in to change notification settings - Fork 26
DOCSP-48356: skipwhile & takewhile #536
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-48356: skipwhile & takewhile #536
Conversation
✅ Deploy Preview for docs-csharp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
shuangela
left a comment
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, one non-blocking suggestion!
source/fundamentals/linq.txt
Outdated
| .. code-block:: csharp | ||
|
|
||
| var query = queryableCollection | ||
| .Select(s => s.Grades.TakeWhile(g => g > 90); |
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.
If the Grades aren't sorted you might be surprised by the 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.
Thanks for the review! Do you think it would better serve users to show a sort on that array in this example?
source/fundamentals/linq.txt
Outdated
| :copyable: false | ||
|
|
||
| { "name" : "Maura Day", "grades" : [92, 97] } | ||
| { "name" : "Saul Curzon", "grades" : [100, 95, 91] } |
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.
The results do NOT resemble these documents.
The result is an array of Grades. It might better to say the results are:
[92, 97]
[100, 95, 91]
Though I have no idea if the values are correct because I don't know the test data.
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! I am going to test this example now that the driver is released. I must have interpreted the test from the source code incorrectly.
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.
Fixed code and results
source/fundamentals/linq.txt
Outdated
| :copyable: false | ||
|
|
||
| { "name" : "Maura Day", "grades" : [80, 90, 70, 83] } | ||
| { "name" : "Saul Curzon", "grades" : [79, 100, 85, 73] } |
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.
The results do NOT resemble these documents.
The result is an array of Grades.
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.
Fixed code and results
source/fundamentals/linq.txt
Outdated
| var query = queryableCollection | ||
| .Select(s => s.Grades.TakeWhile(g => g > 90).ToArray()); | ||
|
|
||
| The results might resemble the following documents: |
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.
The results look right, but calling them "documents" is not quite right.
Maybe just say "The results might resemble the following:"
source/fundamentals/linq.txt
Outdated
| var query = queryableCollection | ||
| .Select(s => s.Grades.SkipWhile(g => g < 75).ToArray()); | ||
|
|
||
| The results might resemble the following documents: |
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.
The results look right, but calling them "documents" is not quite right.
Maybe just say "The results might resemble the following:"
970d0bd to
867c759
Compare
rstam
left a comment
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
* DOCSP-48356: skipwhile & takewhile * copyable false * RStam tech review * RStam tech review 2 (cherry picked from commit c2adeca)
* DOCSP-48356: skipwhile & takewhile * copyable false * RStam tech review * RStam tech review 2
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-48356
Staging Links
linq guide takewhile | skipwhile | table
whats new
Self-Review Checklist