-
Notifications
You must be signed in to change notification settings - Fork 136
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
Verify feature flag and enable prior to deployment #3653
Conversation
remoteProperties[azureWebJobsFeatureFlags] = featureFlagArray.join(','); | ||
} | ||
|
||
context.telemetry.properties.addedRunFromPackage = String(shouldAddSetting); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either remove this line or change addedRunFromPackage
to something more accurate - depending if you want to track this in telemetry
@@ -32,6 +44,8 @@ export async function verifyAppSettings(context: IActionContext, node: SlotTreeI | |||
if (node.site.isLinux) { | |||
const remoteBuildSettingsChanged = verifyLinuxRemoteBuildSettings(context, appSettings.properties, bools); | |||
updateAppSettings ||= remoteBuildSettingsChanged; | |||
} else if (isNodeV4Plus(options)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also applies when isPythonV2Plus
@@ -32,6 +44,8 @@ export async function verifyAppSettings(context: IActionContext, node: SlotTreeI | |||
if (node.site.isLinux) { | |||
const remoteBuildSettingsChanged = verifyLinuxRemoteBuildSettings(context, appSettings.properties, bools); | |||
updateAppSettings ||= remoteBuildSettingsChanged; | |||
} else if (isNodeV4Plus(options)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be its own if
case instead of in an else if
. The new Node.js programming model can be run on both windows and linux
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah doh, obviously.
Fixes #3559