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.

Sign up for GitHub

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

Jump to bottom

CLI: Clean up handling of unknown subcommands #12799

Merged
merged 2 commits into from
Oct 18, 2020
Merged

CLI: Clean up handling of unknown subcommands #12799

merged 2 commits into from
Oct 18, 2020

Conversation

jamesgeorge007
Copy link
Member

@jamesgeorge007 jamesgeorge007 commented Oct 16, 2020 •
edited
Loading

Issue:

What I did

Use commander.js API to handle unknown subcommands.

How to test

This PR aims at leveraging the arg parser lib to handle unknown args.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2020 •
edited
Loading

Fails
🚫

PR is not labeled with one of: ["cleanup","doc-dependencies:update","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies:update","dependencies","other"]

Generated by 🚫 dangerJS against 880d7ee

@ndelangen
Copy link
Member

LGTM, did you test it?

@jamesgeorge007
Copy link
Member Author

LGTM, did you test it?

Yup, works fine 👍
It's nice to have tests for the CLI behavior. As of now, I can just see a shell script for the purpose.

shilman
shilman requested changes Oct 18, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

I ran yarn sb asdf in this branch and also in next and both versions produced the same result. Can you please double check?

Mouse_Highlight_Overlay

@jamesgeorge007
Copy link
Member Author

I ran yarn sb asdf in this branch and also in next and both versions produced the same result.

This PR aims at introducing a refactor, and hence the behavior remains the same.

shilman
shilman reviewed Oct 18, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Aha gotcha. Thanks for the clarification @jamesgeorge007

@shilman shilman added the maintenance User-facing maintenance tasks label Oct 18, 2020
@shilman shilman changed the title refactor: leverage commander.js API to capture unknown subcommands CLI: Clean up handling of unknown subcommands Oct 18, 2020
shilman
shilman approved these changes Oct 18, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

👍

@shilman shilman merged commit 68f6e2c into next Oct 18, 2020
@shilman shilman deleted the refactor/leverage-arg-parser branch October 18, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman approved these changes

@ndelangen ndelangen Awaiting requested review from ndelangen

@stijnkoopal stijnkoopal Awaiting requested review from stijnkoopal

Assignees
No one assigned
Labels
cli maintenance User-facing maintenance tasks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jamesgeorge007 @ndelangen @shilman