-
Notifications
You must be signed in to change notification settings - Fork 337
Disallow templates with generate site #789
Disallow templates with generate site #789
Conversation
0a2ef34
to
3a15bad
Compare
3a15bad
to
8dca047
Compare
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.
some nits but other wise lgtm
src/main.rs
Outdated
let mut target_type = None; | ||
|
||
let template = if site { | ||
if template_value.is_some() { | ||
failure::bail!("You cannot specify a template when generating a Workers Site. If you want to generate site boilerplate, run wrangler generate --site") |
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.
This text is a little confusing to me- I might say:
"You cannot specify a template when generating a Workers Site. If you'd like to use the default site boilerplate, run wrangler generate --site
. If you'd like to use another site boilerplate, omit the --site
when running wrangler generate
."
src/main.rs
Outdated
@@ -390,17 +390,21 @@ fn run() -> Result<(), failure::Error> { | |||
} else if let Some(matches) = matches.subcommand_matches("generate") { | |||
let name = matches.value_of("name").unwrap_or("worker"); | |||
let site = matches.is_present("site"); | |||
let template_value = matches.value_of("template"); |
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.
template value is a confusing name to me- maybe template_url?
src/main.rs
Outdated
let mut target_type = None; | ||
|
||
let template = if site { | ||
if template.is_some() { | ||
failure::bail!("You cannot specify a template when generating a Workers Site. If you'd like to use the default site boilerplate, run wrangler generate --site. If you'd like to use another site boilerplate, omit the --site when running wrangler generate.") |
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.
lol i'm still on this, something seems off here
"You cannot specify a template when generating a Workers Site. If you'd like to use the default site boilerplate, run wrangler generate --site. If you'd like to use another site boilerplate, omit the --site when running wrangler generate."
I think if we change the bolded part to "You've passed both --site
and a template to wrangler generate
- however you can only pass one or the other."
what do you think?
This PR adds failure logic when a template is passed to
wrangler generate --site
Example
Fixes #776