-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add deleteEnvironmentVariable
command
#798
base: mwf/mature-lime
Are you sure you want to change the base?
Conversation
import { type EnvironmentVariablesBaseContext } from "../EnvironmentVariablesContext"; | ||
|
||
export interface EnvironmentVariableDeleteBaseContext extends EnvironmentVariablesBaseContext { | ||
environmentVariable: EnvironmentVar; |
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.
Should this just be part of the edit context?
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.
Just to clarify, which of the edit contexts are you referring to?
}; | ||
wizardContext.telemetry.properties.revisionMode = containerApp.revisionsMode; | ||
|
||
const confirmMessage: string = localize('confirmDeleteEnv', 'Are you sure you want to delete environment variable "{0}"?', item.envVariable.name); |
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.
Should we put a message saying that this isn't destructible (until you upload the draft?)
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 that's a good idea. Maybe I can add (draft)
to the message to help clarify this, do you think that would be clear enough?
…-azurecontainerapps into mwf/running-coffee
No description provided.