Skip to content
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

update help text for log drains #376

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

michaelwang13
Copy link
Member

@michaelwang13 michaelwang13 commented Mar 3, 2025

Consistenly use - for options across log drain creation commands.

Make it more clear what the default behavior is in the help text, since Thor makes it confusing by including the --no- versions for booleans.

Screenshot 2025-03-03 at 11 37 28 AM

Alternatives would be to build Thor ourselves to get rid of it, or manage the default: true ourselves.

@michaelwang13 michaelwang13 requested a review from benjodo March 3, 2025 16:44
Copy link
Member

@UserNotFound UserNotFound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a nit, or i've just seen the word "drain" too much for it to hold any meaning to me anymore...but can we change the help text from "will be drained" to "will be sent to your chosen destination" or something?

@eloisejones
Copy link

Maybe a nit, or i've just seen the word "drain" too much for it to hold any meaning to me anymore...but can we change the help text from "will be drained" to "will be sent to your chosen destination" or something?

I second this suggestion. I think it would help make things clearer.

@michaelwang13
Copy link
Member Author

updated!

Copy link
Member

@UserNotFound UserNotFound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't notice this the first time, but you need to include a space for each \ on all of these help blurbs that were changed.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants