-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
[legacy-framework] Allow user to select a recipe with blitz install
command
#2828
[legacy-framework] Allow user to select a recipe with blitz install
command
#2828
Conversation
Oops, the list of recipes contains tsconfig.json which is not a recipe, I'll fix it. |
Comments that have been pointed out `https://github.com/blitz-js/blitzjs.com/pull/568#pullrequestreview-775673698`
Thanks for your contribution, @mochi-sann! And thanks for the review, @piotrski! |
Accept reviews for blitz-js#2828 (comment) Co-authored-by: Piotrek Tomczewski <[email protected]>
…itz into blitz-cli-add-import_-list
Fixed to use table helper from @blitzjs/display. Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Left minor comments.
Do you think, we could have some tests for that here: https://github.com/blitz-js/blitz/blob/canary/packages/cli/test/commands/install.test.ts#L4?
Wow! So cool! I'll take a look on Monday and review. |
this looks awesome @mochi-sann thank you! |
👍 good stuff |
packages/cli/src/commands/install.ts
Outdated
if (flags.list || !args.recipe) { | ||
const officialRecipeList = await this.getOfficialRecipeList() | ||
recipeInfo = this.normalizeRecipePath(await this.officialRecipeListTable(officialRecipeList)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering do we still need the --list
flag if it's handled in the same way as not passing the arg? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9661bf3
I removed list flag
Co-authored-by: Aleksandra Sikora <[email protected]>
…t flag is present or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works amazing! Thank you a lot for your contribution and sorry for a lot of back and forth with review
I pushed a small change to fix Cloning GitHub repository for undefined recipe
message when selecting a recipe via prompt
You can go ahead and update the docs PR according to the latest changes :) |
blitz install
command
blitz install
commandblitz install
command
Added @mochi-sann contributions for code and test |
blitz install
commandblitz install
command
Closes: blitz-js/legacy-framework#233
What are the changes and their implications?
If a user doesn't provide a recipe name, the command will show a prompt to choose a recipe from a list of available recipes.
Bug Checklist
Feature Checklist
document page pull request