Skip to content

Update DefaultResourcesNameStep#634

Merged
MicroFish91 merged 72 commits intomainfrom
mwf/default-resources-ref
Mar 26, 2024
Merged

Update DefaultResourcesNameStep#634
MicroFish91 merged 72 commits intomainfrom
mwf/default-resources-ref

Conversation

@MicroFish91
Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 commented Mar 19, 2024

Closes #611
Closes #603

  • Removes auto-naming of resources based on the workspace name and always prompts for a name
  • Refactor to leverage each step's validateInput (now static methods)
  • Remove tests that are no longer necessary since logic has become much simpler 🧹

@MicroFish91 MicroFish91 marked this pull request as ready for review March 19, 2024 21:52
@MicroFish91 MicroFish91 requested a review from a team as a code owner March 19, 2024 21:52
@MicroFish91
Copy link
Copy Markdown
Contributor Author

@nturinski Double check this one please

!context.registry && (context.newRegistryName = await RegistryNameStep.tryGenerateRelatedName(context, workspaceName));
!context.containerApp && (context.newContainerAppName = workspaceName);
context.imageName = ImageNameStep.getTimestampedImageName(context.containerApp?.name || workspaceName);
!context.resourceGroup && (context.newResourceGroupName = resourceName);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still suggest we should add a suffix for each resource type.

resourceName-rg
resourceName-me or resourceName-env
resourceName-app or resourceName-aca

I think when you see them all in the resource group, it makes it a lot easier to distinguish what each resource is. I notice other tooling will do that as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like that idea, let me do that in a separate PR after we discuss what that convention should look like

Copy link
Copy Markdown
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Rest looks good to me. So fresh, so clean

Base automatically changed from mwf/remove-dwp to main March 26, 2024 00:03
@MicroFish91 MicroFish91 merged commit 4b8ac71 into main Mar 26, 2024
@MicroFish91 MicroFish91 deleted the mwf/default-resources-ref branch March 26, 2024 00:55
@nturinski nturinski mentioned this pull request Apr 11, 2024
@microsoft microsoft locked and limited conversation to collaborators May 10, 2024
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.

Update default resource naming logic Refactor DefaultResourcesNameStep to utilize each resource step's validateInput

2 participants