Skip to content

Decouple workspace and tree item settings from deployWorkspaceProjectInternal#615

Merged
MicroFish91 merged 6 commits intomainfrom
mwf/dwp-decouple-settings
Mar 13, 2024
Merged

Decouple workspace and tree item settings from deployWorkspaceProjectInternal#615
MicroFish91 merged 6 commits intomainfrom
mwf/dwp-decouple-settings

Conversation

@MicroFish91
Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 commented Mar 5, 2024

Closes #610

  • Decouple workspace and tree item settings from deployWorkspaceProjectInternal
  • Removed options from internal logic since we decoupled this (i.e. they no longer apply): ignoreExistingDeploySettings, suppressProgress.
  • Increment api version

@MicroFish91 MicroFish91 changed the base branch from main to mwf/dwp-housekeeping March 5, 2024 01:23
@MicroFish91 MicroFish91 marked this pull request as ready for review March 5, 2024 01:26
@MicroFish91 MicroFish91 requested a review from a team as a code owner March 5, 2024 01:26
Base automatically changed from mwf/dwp-housekeeping to main March 7, 2024 23:49
import { deployWorkspaceProjectInternal, type DeployWorkspaceProjectInternalContext } from "./internal/deployWorkspaceProjectInternal";

export async function deployWorkspaceProject(context: IActionContext & Partial<DeployWorkspaceProjectContext>, item?: ContainerAppItem | ManagedEnvironmentItem): Promise<DeployWorkspaceProjectResults> {
const subscription: AzureSubscription = await subscriptionExperience(context, ext.rgApiV2.resources.azureResourceTreeDataProvider);
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.

Any reason for moving this?

If we have an item, we can get the subscription information from that, so I feel like it should probably be below that check still. Granted, it doesn't look like we were doing that before, but we probably should have been.

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.

Oh that's a good point, I'll move it back below. And we can consider grabbing the subscription if an item is passed in for the V2 iteration

dwpSettingUtilsV1.displayDeployWorkspaceProjectSettingsOutput(settings);
}
// Logic to display local workspace settings related outputs
if (triggerSettingsOverride(settings, item)) {
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.

Do we still need this logic? I'm assuming that we won't ever overwrite workspace deployment settings anymore.

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.

Not really, but I'm keeping it here because it's necessary to make V1 work which is what is still hooked up until we finish V2 and swap it over :)

@MicroFish91 MicroFish91 merged commit 9e1e86c into main Mar 13, 2024
@MicroFish91 MicroFish91 deleted the mwf/dwp-decouple-settings branch March 13, 2024 18:40
@microsoft microsoft locked and limited conversation to collaborators Apr 28, 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.

Decouple settings validation and parsing logic from deployWorkspaceProjectInternal

2 participants