-
Notifications
You must be signed in to change notification settings - Fork 4
README fixes #42
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
README fixes #42
Conversation
README.rst
Outdated
|
|
||
| ExplainCollection(collection).find({}).sort(...) | ||
|
|
||
| it will fail. |
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.
Can you offer a suggested workaround? Something like:
Instead pass all the arguments to the find() call, like so::
ExplainCollection(collection).find({}, sort=...)
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.
Done.
README.rst
Outdated
| - the explain output will be sent to whatever stream your script configures the logging module to send output to. | ||
| - if your script sets the `logging level <https://docs.python.org/3/library/logging.html#logging-levels>`_ | ||
| higher than INFO, the explain output will be suppressed entirely. | ||
| - the explain output will be sent to whatever stream your script configures the logging module to send output to. |
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.
This still doesn't look correct in the rendered view: https://github.com/mongodb-labs/pymongoexplain/blob/b4bd835c930b496f0641a6a81f511244a21c179e/README.rst#explaining-commands-in-a-script
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.
Done.
README.rst
Outdated
|
|
||
| Limitations | ||
| ----------- | ||
| This package does not support the fluent `Cursor API <https://api.mongodb.com/python/current/api/pymongo/cursor.html>`_, |
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.
This is not the correct link for PyMongo documentation. It should be: https://pymongo.readthedocs.io/en/stable/api/pymongo/cursor.html
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.
Done.
| ================== | ||
|
|
||
| For questions, discussions, or general technical support, visit the MongoDB Community Forums. | ||
| For questions, discussions, or general technical support, visit the `MongoDB Community Forums <https://developer.mongodb.com/community/forums/tag/python>`_. |
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.
Please add a link to this GitHub repository in the reference to 'this GitHub repository' on L48
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.
Done.
|
|
||
|
|
||
| Limitations | ||
| ----------- |
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.
Please leave a blank line after the title.
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.
Done.
prashantmital
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.
Please change the import to ExplainableCollection.
ShaneHarvey
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
No description provided.