-
Notifications
You must be signed in to change notification settings - Fork 29
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
docs: improve doc strings text #436
Conversation
Manage AWS Deadline Cloud's workstation configuration. Config options are organized | ||
and documented in config_file.SETTINGS. |
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.
What steps does a user take to then figure this information from the help text? Do they have to go code spelunking? I wonder if we can avoid that. Example:
(deadline) bash-3.2$ deadline config
Usage: deadline config [OPTIONS] COMMAND [ARGS]...
Manage AWS Deadline Cloud's workstation configuration. Config options are
organized and documented in config_file.SETTINGS.
Options:
-h, --help Show this message and exit.
Commands:
get Gets an AWS Deadline Cloud workstation configuration setting.
gui Open the workstation configuration settings GUI.
set Sets an AWS Deadline Cloud workstation configuration setting.
show Show AWS Deadline Cloud's current workstation configuration.
If we do change this, the get and set command will also have to change:
(deadline) bash-3.2$ deadline config set --help
Usage: deadline config set [OPTIONS] SETTING_NAME VALUE
Sets an AWS Deadline Cloud workstation configuration setting.
For example `deadline config set defaults.farm_id <farm-id>`. Run `deadline
config --help` to show available settings.
Options:
-h, --help Show this message and exit.
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.
Wrapping the documentation into deadline config show
.
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.
Excerpt of sample output:
> deadline config show
AWS Deadline Cloud configuration file:
/Users/crowest/.deadline/config
deadline-cloud-monitor.path: ...
The filesystem path to Deadline Cloud monitor, set during login process.
defaults.aws_profile_name: ...
The AWS profile name to use by default. Set to '' to use the default
credentials. Other settings are saved with the profile.
settings.job_history_dir: ~/.deadline/job_history/ ... (default)
The directory in which to place the job submission history for this AWS
profile name.
defaults.farm_id: farm-281174...
The Farm ID to use by default.
settings.storage_profile_id: (default)
The storage profile that this workstation conforms to. It specifies where
shared file systems are mounted, and where named job attachments should go.
defaults.queue_id: queue-38906...
The Queue ID to use by default.
"[1] SKIP: Do not download these files\n" | ||
"[2] OVERWRITE: Download these files and overwrite existing files\n" | ||
"[3] CREATE_COPY: Download the file with a new name, appending '(1)' to the end", | ||
help="The resolution method to use when a file already exists:\n" |
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.
starting with resolution feels odd to me, but I guess that's what the option is named. "How to handle existing files" might be a shorter way to put it. I think it's probably fine though. Just musing out loud.
Change looks good, just a few suggestions. 2 additional things that don't get captured in the diff:
for example: deadline isn't truncated but there's also a lot of self-referential
but
same with other ones like
Might make sense to include that here in this change? |
Signed-off-by: Stephen Crowe <[email protected]>
Signed-off-by: Stephen Crowe <[email protected]>
Quality Gate passedIssues Measures |
Sample output from
|
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. Ship it!
What was the problem/requirement? (What/Why)
The client includes help text for its commands, and some of the help text could be improved.
What was the solution? (How)
Update help text as needed.
Fixes #324
Fixes #365
What is the impact of this change?
Someone can more easily understand how to use the client.
How was this change tested?
Have you run
hatch run test
?Yes
Have you run
hatch run integ:test
? SeeDEVELOPMENT.md
on "Run integration tests"No
Was this change documented?
Yes
Is this a breaking change?
No
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.