Skip to content
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 Execute Step in createFunction API #3150

Merged
merged 4 commits into from
May 5, 2022

Conversation

VasuBhog
Copy link
Contributor

@VasuBhog VasuBhog commented May 4, 2022

This additional API property would allow users to execute logic in relation to the azure function project they are creating prior to the OpenFolderStep by setting the priority accordingly.

This will aid in the creation of Azure Function with SQL Bindings as a user will need to create new azure function project and the logic to add SQL connection string to local.settings.json would not run due to the vscode extension host reloading based on the user choosing to Open in current window or Open in new window:
Screen Shot 2022-04-22 at 1 03 10 PM

This would benefit our users greatly and would be a much-improved user experience as a whole.

Tested this locally via microsoft/azuredatastudio#19293.

@VasuBhog VasuBhog marked this pull request as ready for review May 5, 2022 07:29
@VasuBhog VasuBhog requested a review from a team as a code owner May 5, 2022 07:29
@@ -64,7 +65,7 @@ export async function createNewProjectInternal(context: IActionContext, options:
const wizard: AzureWizard<IFunctionWizardContext> = new AzureWizard(wizardContext, {
title: localize('createNewProject', 'Create new project'),
promptSteps: [new FolderListStep(), new NewProjectLanguageStep(options.templateId, options.functionSettings), new OpenBehaviorStep()],
executeSteps: [new OpenFolderStep()]
...(optionalExecuteStep !== undefined ? { executeSteps: [optionalExecuteStep, new OpenFolderStep()] } : { executeSteps: [new OpenFolderStep()] }),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just do
executeSteps: optionalExecuteStep ? [optionalExecuteStep, new OpenFolderStep()] : [new OpenFolderStep()]

/**
* If set, it will include a step that will be executed prior to OpenFolderStep
*/
executeStep?: AzureWizardExecuteStep<IActionContext>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest something clearer indicating that this is specifically right before the folder is opened. Maybe beforeOpenFolderExecuteStep

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, ok nevermind. Forgot that the order is based on the priority instead. In that case this is fine - although you should update the comment to indicate that.

Copy link
Contributor

Choose a reason for hiding this comment

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

This still isn't accurate - it's not guaranteed to be before the OpenFolderStep. I'd just remove that and say executed in the order determined by the priority of the step

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could include the priority number of OpenFolderStep so people know to set the priority lower than that (lower in this case is higher priority)

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking that - but then if the priority is ever changed it's slightly annoying to have to remember to update this comment as well. Maybe do that but also provide a link to the code line so it's easy to verify as well?

@nturinski
Copy link
Member

You should bump the apiVersion here as well https://github.com/microsoft/vscode-azurefunctions/blob/main/src/extension.ts#L102

Since older versions of the extension wouldn't support the new option, you can check the user has the right version and alert them to upgrade the extension. Here's an example: https://github.com/microsoft/vscode-azurestaticwebapps/blob/main/src/getExtensionApi.ts#L17-L30

Copy link
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.

Looks good! Thanks for the contribution. Always awesome when other teams help out 😊

@nturinski nturinski merged commit 8694350 into microsoft:main May 5, 2022
@Charles-Gagnon
Copy link
Contributor

@nturinski When can we expect a new version of the extension to be published with this change?

@nturinski
Copy link
Member

We're planning to release in a couple of weeks.

nturinski pushed a commit that referenced this pull request May 19, 2022
* add execute step in api to allow for execution of new azure function prior to openFolderStep

* address comments

* vbump apiVersion

* add comment
@microsoft microsoft locked and limited conversation to collaborators Jun 20, 2022
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.

3 participants