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

feat: add accept-defaults to new templates #695

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

karthik2804
Copy link
Contributor

@karthik2804 karthik2804 commented Aug 17, 2022

Add optional argument --accept-defaults to spin new to accept defaults where available from the template. The values file and value mentioned in the CLI still take precedence.

closes #591.

@karthik2804 karthik2804 force-pushed the feat/accept-defaults branch 2 times, most recently from 0f62200 to d5a88ea Compare August 17, 2022 16:20
src/commands/new.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on. I'd strongly encourage you to dive a little deeper into the templates crate - although it requires more code reading, I believe you'll end up at an existing function where the code has both the parameter object and designated value to hand, and is performing the existing "value > prompt" logic. Extending that to "value > accepted default > prompt" should (I hope) be a couple of lines.

Happy to chat or pair I'm not explaining myself well...

src/commands/new.rs Outdated Show resolved Hide resolved
src/commands/new.rs Outdated Show resolved Hide resolved
src/commands/new.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

This looks good - thanks! Couple of minor recommendations, but otherwise good to go!

crates/templates/src/manager.rs Outdated Show resolved Hide resolved
crates/templates/src/run.rs Outdated Show resolved Hide resolved
crates/templates/src/run.rs Outdated Show resolved Hide resolved
Signed-off-by: karthik Ganeshram <karthik.ganeshram@fermyon.com>
Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

LGTM!

@radu-matei radu-matei merged commit 6bbf268 into fermyon:main Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spin new: flag to accept defaults where available
3 participants