-
Notifications
You must be signed in to change notification settings - Fork 233
Update the default
azd metadata field's type to any
#5384
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
base: main
Are you sure you want to change the base?
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.
LGTM - @vhvb1989, will adding this flexibility cause any unintended issues elsewhere?
FYI @alexwolfmsft for updating the docs |
defaultBool := true | ||
if azdMetadata.Default != nil { | ||
switch v := azdMetadata.Default.(type) { | ||
case bool: | ||
defaultBool = v | ||
case string: | ||
defaultBool = strings.EqualFold(v, "true") | ||
default: | ||
strVal := fmt.Sprintf("%v", v) | ||
defaultBool = strings.EqualFold(strVal, "true") | ||
} | ||
} | ||
msg := fmt.Sprintf("Would you like to set the '%s' infrastructure %s to %t?", key, securedParam, defaultBool) | ||
confirmValue, err := p.console.Confirm(ctx, input.ConsoleOptions{ | ||
Message: msg, | ||
Help: help, | ||
Options: options, | ||
DefaultValue: defaultValueForPrompt, | ||
DefaultValue: true, | ||
}) |
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 feel like the new prompt change could get a bit confusing, especially if the Bicep param has a default of false
because of the double negative.
Bicep:
@metadata({azd: {
default: false
}})
param param1 bool
New prompt: if the user inputs 'n', then the value becomes true
in this case:

Old prompt, a bit more explicit:
@SophCarp would love to get your thoughts here as well 🙂
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 prefer the way of selection (y/n) in the new prompt, but it does feel confusing due to double negative. Could we ask if users would like to set 'paraml' to true (y/N)?
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.
hmmm. By default, param1 would be false. y/n isn't the same as true/false, but rather "confirm/change". "default" should be included in the question, because otherwise a customer might be confused why they're being asked a double negative question.
Maybe :
? 'param1's default value is false. Keep default value? y/n
or:
? Infrastructure parameter 'param1' has a default value of 'false'. Keep default value?
> Keep (param1=false)
Change (param1=true)
or:
? Confirm infrastructure parameter 'param1' = false:
> Yes (param1=false)
No (param1=true)
or:
? Confirm infrastructure parameter 'param1' = false: y/n
Is there more direct wording than "infrastructure parameter"? This is different from "environment variable", right?
used #5386 to fix the prompt regressions on bool/int with no defaults. @Menghua1 , let's use this PR for only adding support for types in the default azd metadata field. Instead of the current: Similar for |
Remove temporary variables Co-authored-by: JeffreyCA <[email protected]>
bool
default in AzdMetadata
and use Confirmation
for boolean parametersdefault
azd metadata field's type to any
@vhvb1989 Got it. The code has been updated to only add support for types in the default azd metadata field. |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
This PR updates the
AzdMetadata.Default
field type toany
to support default values of typebool
andint
.For

bool
parameters: TheAzdMetadata.Default
field must be of typebool
.Other types are not supported and will result in an error as follow:

For

int
parameters: TheAzdMetadata.Default
field must be of typeint
.Other types are not supported and will result in an error as follow:

@rajeshkamal5050 and @vhvb1989 for notification.