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

Start re-factoring the CLI codebase #177

Merged
merged 1 commit into from
Jul 3, 2018
Merged

Conversation

RomainMuller
Copy link
Contributor

Breaking down the monolithic CLI infrastructure by using yarg's commandDir directive (see #176). This allows modelling each command in a separate module for a cleaner interface.

Migrated the docs command, and created a prototype of a doctor command (see #154). The new commands also have basic unit tests that verify the handlers honor their promises.

@RomainMuller RomainMuller requested review from rix0rrr and eladb June 25, 2018 21:31
@RomainMuller RomainMuller force-pushed the rmuller/new-cli branch 2 times, most recently from c09ad3f to 2684cc4 Compare June 25, 2018 23:01
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

I think “commands” should be under “lib”

}

const verifications: Array<() => boolean> = [
displayVersionInformation,
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to make these asynchronous since you will potentially have some file I/O to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense!

@@ -0,0 +1,60 @@
import * as mockery from 'mockery';
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note that this must be required first

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 can be required whenever... But I guess if you want to use it, you have to not require the stuff you want it to change how they are required before you've configured it... Which feels kind of obvious...

RomainMuller added a commit that referenced this pull request Jul 3, 2018
Breaking down the monolithic CLI infrastructure by using `yarg`'s
`commandDir` directive (see #176). This allows modelling each command in
a separate module for a cleaner interface.

Migrated the `docs` command, and created a prototype of a `doctor`
command (see #154). The new commands also have basic unit tests that
verify the handlers honor their promises.
Breaking down the monolithic CLI infrastructure by using `yarg`'s
`commandDir` directive (see #176). This allows modelling each command in
a separate module for a cleaner interface.

Migrated the `docs` command, and created a prototype of a `doctor`
command (see #154). The new commands also have basic unit tests that
verify the handlers honor their promises.
@RomainMuller RomainMuller merged commit 8db8ad9 into master Jul 3, 2018
@RomainMuller RomainMuller deleted the rmuller/new-cli branch July 3, 2018 11:14
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants