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

Provide Docs URLs in --help messaging #1232

Merged
merged 4 commits into from
Oct 19, 2022
Merged

Provide Docs URLs in --help messaging #1232

merged 4 commits into from
Oct 19, 2022

Conversation

dacbd
Copy link
Contributor

@dacbd dacbd commented Oct 19, 2022

I did some very basic skimming of yargs to see if they had a better mechanism for this but didn't find anything (note on the basic skimming, I find their docs website difficult to read and search)

@dacbd dacbd temporarily deployed to internal October 19, 2022 00:12 Inactive
Copy link
Contributor

@tasdomas tasdomas left a comment

Choose a reason for hiding this comment

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

I wonder if we need either a test or a cron action to validate that none of the links are dead.

@dacbd
Copy link
Contributor Author

dacbd commented Oct 19, 2022

That should be easy enough to add

@dacbd dacbd temporarily deployed to internal October 19, 2022 15:24 Inactive
@dacbd dacbd requested a review from tasdomas October 19, 2022 16:01
@dacbd dacbd self-assigned this Oct 19, 2022
@dacbd dacbd added the ui/ux User interface/experience label Oct 19, 2022
@dacbd dacbd requested a review from a team October 19, 2022 16:02
@dacbd dacbd marked this pull request as ready for review October 19, 2022 16:02
bin/cml.test.js Outdated Show resolved Hide resolved
@dacbd dacbd temporarily deployed to internal October 19, 2022 16:03 Inactive
Comment on lines +38 to +46
'workflow/rerun',
'tensorboard/connect',
'runner/launch',
'repo/prepare',
'pr/create',
'comment/create',
'comment/update',
'check/create',
'asset/publish'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'workflow/rerun',
'tensorboard/connect',
'runner/launch',
'repo/prepare',
'pr/create',
'comment/create',
'comment/update',
'check/create',
'asset/publish'
'workflow:rerun',
'tensorboard:connect',
'runner:launch',
'repo:prepare',
'pr:create',
'comment:create',
'comment:update',
'check:create',
'asset:publish'

For consistency with analytics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only used to import the file, we could but it would and more code in the test :/

Copy link
Member

Choose a reason for hiding this comment

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

My bad, nevermind

Copy link
Contributor

Choose a reason for hiding this comment

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

going to be hard to keep this list updated as (sub)commands new are added.

@dacbd dacbd merged commit ee8d4c5 into master Oct 19, 2022
@dacbd dacbd deleted the help-messaging branch October 19, 2022 17:59
@dacbd dacbd mentioned this pull request Oct 20, 2022
tasdomas pushed a commit that referenced this pull request Oct 28, 2022
* initial setup

* npm run do_snapshots

* now with a test

* Update bin/cml.test.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/ux User interface/experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants