-
Notifications
You must be signed in to change notification settings - Fork 337
feat: adds more descriptive subdomain errors #259
feat: adds more descriptive subdomain errors #259
Conversation
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.
looks great! merge at will once rebased :)
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.
i think this looks good- i want to discuss the subdomain "info/success" message a little more as i think it might be confusing, but otherwise i think we are good to go!
src/commands/subdomain.rs
Outdated
); | ||
if sd == name { | ||
println!( | ||
"{} Your subdomain {}.workers.dev has already been registered!", |
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.
I would read this as "Your subdomain has already been registered (by someone else)" – is that the intent here? If not, should it be something like "You've already registered the subdomain xyz.workers.dev"?
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.
Yeah so this case is specifically for when the user is trying to register the subdomain that they themselves have already registered. The latter makes sense to me
Warns users when attempting to add a subdomain w/o account_id in wrangler.toml Doesn't show failure when attempting to register the subdomain that is already associated with their account
Warns users when attempting to add a subdomain w/o account_id in
wrangler.toml
Doesn't show failure when attempting to register the subdomain that
is already associated with their account
Fixes #207