Conversation
- `-o` and `-O` help: say what each option does to the value, and give an example. "Raw strings" and "interpreted strings" did not tell users which one to use. - `migrate -v`: add help text. - `upload_dir`: remove the double space in `Uploading directory`. - `sync --pull`: add the folder name to `Preparing to download N items from Cloudinary folder`, which ended with a space. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TalLevAmi
marked this pull request as ready for review
September 26, 2026 16:16
const-cloudinary
requested changes
Sep 27, 2026
const-cloudinary
left a comment
const-cloudinary
left a comment
Member
There was a problem hiding this comment.
@TalLevAmi, please improve description of some of the parameters, they can be misleading, especially for naive LLMs
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") | ||
| @option("-O", "--optional_parameter_parsed", multiple=True, nargs=2, | ||
| help="Pass optional parameters as interpreted strings.") | ||
| help="Pass an optional parameter and parse its value as JSON or a boolean. e.g. -O context '{\"alt\": \"cat\"}'") |
Member
There was a problem hiding this comment.
Example is misleading, provisioning commands do not support context parameter.
| @argument("params", nargs=-1) | ||
| @option("-o", "--optional_parameter", multiple=True, nargs=2, help="Pass optional parameters as raw strings.") | ||
| @option("-o", "--optional_parameter", multiple=True, nargs=2, | ||
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") |
Member
There was a problem hiding this comment.
Example is misleading, provisioning commands do not support tags parameter.
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") | ||
| @option("-O", "--optional_parameter_parsed", multiple=True, nargs=2, | ||
| help="Pass optional parameters as interpreted strings.") | ||
| help="Pass an optional parameter and parse its value as JSON or a boolean. e.g. -O context '{\"alt\": \"cat\"}'") |
Member
There was a problem hiding this comment.
Example is misleading, utils commands do not support context parameter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Small text fixes found in a usability review of the CLI.
Brief Summary of Changes
-o, --optional_parameterand-O, --optional_parameter_parsedinadmin,uploader,provisioning,utils,upload_dirandsync: the help says that-odoes not parse the value and that-Oparses it as JSON or a boolean, with an example for each. Before, the help said "raw strings" and "interpreted strings".migrate -v: add help text (Log each migrated URL.).upload_dir: the log line wasUploading directory '...'(two spaces) when-ewas not set.sync --pull: the log linePreparing to download N items from Cloudinary folderhad no folder name. It now shows the folder, as the othersynclines do.What does this PR address?
Are tests included?
Reviewer, please note:
Checklist:
Local test run: I ran the full suite on Python 3.8 with Click 8.1.8. No test asserts these strings. 8 tests in
test_cli_agent.pyandtest_cli_config_oauth.pyfail withstderr not separately capturedon this branch and onmastertoo. These tests need Click 8.2, which needs Python 3.10 or later. CI must confirm the full result.🤖 Generated with Claude Code