-
Notifications
You must be signed in to change notification settings - Fork 249
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
Clarify language around "max" CLI flags #38
Conversation
I've attempted to be more explicit and consistent with the language used to explain and differentiate between the two CLI flags: ``--max-results`` and ``--results-per-call``. These are understandably a bit confusing and #37 reminded me that we could try to be more specific.
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'm tagging @EmilySheehan here as we discussed changing results-per-call
and max-results
of these to not be overloaded with the search API at all, but I like adding the explicit notice here. Thanks!
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.
Whoops, I made a mistake - these changes should go into the examples/base_readme.rst
file, as this readme is generated from there. @jrmontag, do you mind putting this verbiage into that file?
Thanks for the clarification, and no problem. I'll make those changes and add another commit. |
@binaryaaron I've reverted that change and move the text into the correct file. Let me know if you'd like to see any other changes. |
@jrmontag @binaryaaron could you guys follow up on this? |
I've attempted to be more explicit and consistent with the language used to explain and differentiate between the two CLI flags:
--max-results
and--results-per-call
. These are understandably a bit confusing and #37 reminded me that we could try to be more specific.