Skip to content

Remove the 'Edit Secret Name..." command#501

Merged
motm32 merged 3 commits intomainfrom
meganmott/fix-475
Oct 19, 2023
Merged

Remove the 'Edit Secret Name..." command#501
motm32 merged 3 commits intomainfrom
meganmott/fix-475

Conversation

@motm32
Copy link
Copy Markdown
Contributor

@motm32 motm32 commented Oct 17, 2023

Fixes #475 and #489

@motm32 motm32 requested a review from a team as a code owner October 17, 2023 20:00
@motm32 motm32 changed the title Show previous secret name as a placeholder when using the 'Edit Secret Name..." command Remove the 'Edit Secret Name..." command Oct 18, 2023
@motm32
Copy link
Copy Markdown
Contributor Author

motm32 commented Oct 18, 2023

After some discussion with Anthony we decided it was better to remove "Edit Secret Name..." all together.

Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 left a comment

Choose a reason for hiding this comment

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

Some nit comments below, otherwise looks good :)

public async prompt(context: ISecretContext): Promise<void> {
context.newSecretName = (await context.ui.showInputBox({
prompt: localize('secretName', 'Enter a secret name.'),
placeHolder: context.secretName ?? localize('emptyPlaceholder', ''),
Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 Oct 18, 2023

Choose a reason for hiding this comment

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

Nit: We can probably remove the placeHolder since we won't be editing existing secret names anymore

context.activityTitle = localize('updatingSecretName', 'Update secret name from "{0}" to "{1}" in container app "{2}"', context.secretName, context.newSecretName, containerApp.name);
updatedSecret = localize('updatedSecretName', 'Updated secret name from "{0}" to "{1}" in container app "{2}".', context.secretName, context.newSecretName, containerApp.name);
} else if (context.newSecretValue) {
if (context.newSecretValue) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An alternative that might make more sense now that we won't be updating secret names anymore - maybe we can just rename this entire step to something like SecretValueUpdateStep? Then we could drop the if statement entirely so that it just always runs that section of code when the step is called

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh can probably also remove let updatedSecret... (line 21/23) and move the string down to the bottom where it's used now as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Suggestion] It would be better to show the previous name in the input box when executing "Edit Secret Name..." action

2 participants