Skip to content

perf: use prompts instead of inquirer #5443

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

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

janoshrubos
Copy link
Contributor

PR Checklist

What is the current behavior?

Using inquirer package

What is the new behavior?

Using prompts package instead of inquirer in the prompter.

@cla-bot cla-bot bot added the cla: yes label Dec 1, 2020
@NathanaelA

This comment was marked as abuse.

@NathanaelA NathanaelA marked this pull request as draft December 1, 2020 18:25
@janoshrubos
Copy link
Contributor Author

Yes, @rigor789 had the idea to replace the packages to reduce the size of the cli.

@NathanaelA NathanaelA marked this pull request as ready for review December 1, 2020 21:50
@rigor789 rigor789 self-assigned this Dec 2, 2020
@rigor789
Copy link
Member

rigor789 commented Dec 2, 2020

Yep the goal was to reduce the CLI size!

Before (7.0.11):
239 369 798 bytes (286,7 MB on disk) for 17 350 items

After (this PR):
233 977 884 bytes (268,9 MB on disk) for 13 576 items

Diff:
-17,8 MB on disk
-3774 files

I think that's a decent reduction from a relatively small change!

/cc @NathanaelA @NathanWalker

Copy link
Contributor

@NathanaelA NathanaelA left a comment

Choose a reason for hiding this comment

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

Looking at the code, I see nothing wrong and reducing the CLI dependencies is a good thing. Thanks!

LGTM...

@rigor789 rigor789 added this to the 7.0.12 milestone Dec 2, 2020
@rigor789 rigor789 merged commit ab24ece into NativeScript:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants