Skip to content

Added a more generic deploy command #277

Merged
motm32 merged 7 commits intomainfrom
meganmott/genericDeploy
Mar 16, 2023
Merged

Added a more generic deploy command #277
motm32 merged 7 commits intomainfrom
meganmott/genericDeploy

Conversation

@motm32
Copy link
Copy Markdown
Contributor

@motm32 motm32 commented Mar 1, 2023

Fixes #275.

Added "Deploy Container App..." to the cloud button in the workspace.
image

Also added a prompt for different deployment methods.
image

@MicroFish91
Copy link
Copy Markdown
Contributor

MicroFish91 commented Mar 1, 2023 •
edited
Loading

  • One thing I've been thinking about is if there's a way we can refactor the existing ImageSourceListStep to be reusable here. It does what we want mostly, which is to prompt for and set context.imageSource. The main issue is that it currently doesn't support a way to filter out QuickStartImage....
  • Building on that, since right now we only support one option for deploy, we can probably auto set the context.imageSource option until we implement the other methods 🙂

bwateratmsft
bwateratmsft reviewed Mar 3, 2023
View reviewed changes
Comment thread src/commands/deployImage/deploy.ts Outdated
@motm32
Copy link
Copy Markdown
Contributor Author

motm32 commented Mar 3, 2023

Based on offline discussion a couple of changes were made:

  • Deploy now utilizes ImageSourceListStep and the quickstart image pick is filtered out when deploying
  • The first step in deploy is now choosing a container app instead of method of deployment to be consistent with our other extensions
  • Added a "Update an image in container app" pick when deploying if an image is detected

MicroFish91
MicroFish91 reviewed Mar 6, 2023
View reviewed changes
Comment thread src/commands/createContainerApp/createContainerApp.ts Outdated
Comment thread src/commands/createContainerApp/ImageSourceListStep.ts Outdated
Comment thread src/commands/deployImage/IDeployImageContext.ts Outdated
Comment thread src/commands/createContainerApp/ImageSourceListStep.ts
Comment thread src/commands/createContainerApp/ImageSourceListStep.ts Outdated
Comment thread src/commands/deployImage/deploy.ts Outdated
MicroFish91
MicroFish91 reviewed Mar 7, 2023
View reviewed changes
Comment thread src/commands/deployImage/deploy.ts Outdated
Comment thread src/commands/deployImage/deploy.ts Outdated
Comment thread package.json
@motm32 motm32 marked this pull request as ready for review March 15, 2023 19:12
@motm32 motm32 requested a review from a team as a code owner March 15, 2023 19:12
MicroFish91
MicroFish91 reviewed Mar 16, 2023
View reviewed changes
Comment thread src/commands/deploy/ImageSourceListStep.ts Outdated
MicroFish91
MicroFish91 reviewed Mar 16, 2023
View reviewed changes
Comment thread src/commands/deploy/IDeployBaseContext.ts Outdated
MicroFish91
MicroFish91 approved these changes Mar 16, 2023
View reviewed changes
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.

I left 2 comments above to look at.

Otherwise, looks good 🙂🚀

alexweininger
alexweininger approved these changes Mar 16, 2023
View reviewed changes
Comment thread src/commands/deploy/deploy.ts Outdated
targetContainer: containerApp
};

const title: string = localize('deploy', 'Deploying to "{0}"', containerApp.name);
Copy link
Copy Markdown
Member

@alexweininger alexweininger Mar 16, 2023

Choose a reason for hiding this comment

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

It looks like this variable is only used in one place below on line 44. Can we just create this value inline on line 44 instead of creating a constant for it here?

@motm32 motm32 merged commit ecade3c into main Mar 16, 2023
@motm32 motm32 deleted the meganmott/genericDeploy branch March 16, 2023 17:29
@microsoft microsoft locked and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nturinski nturinski nturinski left review comments

@alexweininger alexweininger alexweininger approved these changes

@bwateratmsft bwateratmsft bwateratmsft approved these changes

@MicroFish91 MicroFish91 MicroFish91 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add a more generic deploy command

5 participants

@motm32 @MicroFish91 @nturinski @alexweininger @bwateratmsft
⚡