This repository has been archived by the owner on Dec 9, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 161
fix: Fix typing errors in ARM params and add interfaces #347
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pjlittle
reviewed
Sep 27, 2019
storageAccountName: { | ||
value: StorageAccountResource.getResourceName(config), | ||
}, | ||
storageAccountSkuName: { | ||
value: resourceConfig.sku.name, | ||
}, | ||
storageAccoutSkuTier: { | ||
storageAccountSkuTier: { |
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, the small things...
pjlittle
reviewed
Sep 27, 2019
[key: string]: ArmParameter; | ||
} | ||
|
||
export interface ArmParameter { |
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.
thx for making the abstractions clearer.
pjlittle
approved these changes
Sep 27, 2019
PIC123
approved these changes
Sep 27, 2019
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.
Looks good!
wbreza
pushed a commit
that referenced
this pull request
Oct 6, 2019
tbarlow12
added a commit
that referenced
this pull request
May 7, 2020
* feat: ConfigService for centralizing configuration and simplifying BaseService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration. * release: Update prerelease version to 1.0.2-0 ***NO_CI*** * fix: Sync triggers on external package deployment (#339) Fix updating of function app settings (SDK call wasn't working) and syncing triggers for function apps running from external package. * release: Update prerelease version to 1.0.2-1 ***NO_CI*** * ci: fix failing Node 8 builds on windows agent (#345) Hosted agent roll out a fix that broke our builds: 1. Previously, npm wasn’t getting packaged with the version of node in the tool cache, ie. npm 5.6.0 should be used alongside Node 8.10.0. 1. The fix is to pin to a later version of Node 8 (e.g. 8.16.1) which comes with npm 6+ - https://nodejs.org/en/download/releases/. * This will probably slow the build down a little bit since the agent will have to download the version (instead of it being pre-installed), but we'll get the right version of npm for free. * fix job name * fix job name restrictiosn * still trying to get the right job name format * clean up job name * release: Update prerelease version to 1.0.2-2 ***NO_CI*** * release: Update prerelease version to 1.0.2-3 ***NO_CI*** * fix: Fix typing errors in ARM params and add interfaces (#347) * release: Update prerelease version to 1.0.2-4 ***NO_CI*** * fix: Update to support CosmosDB bindings (#350) Updatings the binding schema that is generated to support the changes made to Cosmos DB * release: Update prerelease version to 1.0.2-5 ***NO_CI*** * feat: Refactor runtime configuration to allow for non-node runtimes (#348) - Added `FunctionRuntime` configuration to provider - Extracting `FunctionRuntime` from `runtime` property of configuration within `ConfigService` - Refactored node-specific code in ARM template generation * release: Update prerelease version to 1.0.2-6 ***NO_CI*** * fix: Update GitHub Issue and PR templates (#353) * release: Update prerelease version to 1.0.2-7 ***NO_CI*** * fix: Sort deployments in descending order and fix APIM arm template (#354) - Updated parameter name in APIM arm template - Fixed bug of sorting deployments in ascending order, when it should have been descending. This would have pretty serious consequences, because it means that the comparison of ARM templates would always be targeting the first ever deployment, not the most recent. - Because the `sort()` function sorts the array in place, this bug was not being detected by the tests. Updated `resourceService` tests to create copies of the array rather than using the original reference when testing the validity of the result. * release: Update prerelease version to 1.0.2-8 ***NO_CI*** * release: Update patch version to 1.0.2 ***NO_CI*** * ci: Add GitHub workflow to move new issues to "To triage" column (#381) * build(deps): bump handlebars from 4.1.2 to 4.5.3 (#400) Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3) Signed-off-by: dependabot[bot] <support@github.com> * Fix displayName for cosmosDBTrigger (#399) * Update bindings.json Co-authored-by: sls-az@microsoft.com <Serverless Azure Functions> Co-authored-by: My <mydiemho@users.noreply.github.com> Co-authored-by: Wallace Breza <wallace@breza.me> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ofek Bashan <ofekray@gmail.com>
tbarlow12
added a commit
that referenced
this pull request
May 12, 2020
* feat: ConfigService for centralizing configuration and simplifying BaseService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration. * release: Update prerelease version to 1.0.2-0 ***NO_CI*** * fix: Sync triggers on external package deployment (#339) Fix updating of function app settings (SDK call wasn't working) and syncing triggers for function apps running from external package. * release: Update prerelease version to 1.0.2-1 ***NO_CI*** * ci: fix failing Node 8 builds on windows agent (#345) Hosted agent roll out a fix that broke our builds: 1. Previously, npm wasn’t getting packaged with the version of node in the tool cache, ie. npm 5.6.0 should be used alongside Node 8.10.0. 1. The fix is to pin to a later version of Node 8 (e.g. 8.16.1) which comes with npm 6+ - https://nodejs.org/en/download/releases/. * This will probably slow the build down a little bit since the agent will have to download the version (instead of it being pre-installed), but we'll get the right version of npm for free. * fix job name * fix job name restrictiosn * still trying to get the right job name format * clean up job name * release: Update prerelease version to 1.0.2-2 ***NO_CI*** * release: Update prerelease version to 1.0.2-3 ***NO_CI*** * fix: Fix typing errors in ARM params and add interfaces (#347) * release: Update prerelease version to 1.0.2-4 ***NO_CI*** * fix: Update to support CosmosDB bindings (#350) Updatings the binding schema that is generated to support the changes made to Cosmos DB * release: Update prerelease version to 1.0.2-5 ***NO_CI*** * feat: Refactor runtime configuration to allow for non-node runtimes (#348) - Added `FunctionRuntime` configuration to provider - Extracting `FunctionRuntime` from `runtime` property of configuration within `ConfigService` - Refactored node-specific code in ARM template generation * release: Update prerelease version to 1.0.2-6 ***NO_CI*** * fix: Update GitHub Issue and PR templates (#353) * release: Update prerelease version to 1.0.2-7 ***NO_CI*** * fix: Sort deployments in descending order and fix APIM arm template (#354) - Updated parameter name in APIM arm template - Fixed bug of sorting deployments in ascending order, when it should have been descending. This would have pretty serious consequences, because it means that the comparison of ARM templates would always be targeting the first ever deployment, not the most recent. - Because the `sort()` function sorts the array in place, this bug was not being detected by the tests. Updated `resourceService` tests to create copies of the array rather than using the original reference when testing the validity of the result. * release: Update prerelease version to 1.0.2-8 ***NO_CI*** * release: Update patch version to 1.0.2 ***NO_CI*** * ci: Add GitHub workflow to move new issues to "To triage" column (#381) * build(deps): bump handlebars from 4.1.2 to 4.5.3 (#400) Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3) Signed-off-by: dependabot[bot] <support@github.com> * Fix displayName for cosmosDBTrigger (#399) * Update bindings.json Co-authored-by: sls-az@microsoft.com <Serverless Azure Functions> Co-authored-by: My <mydiemho@users.noreply.github.com> Co-authored-by: Wallace Breza <wallace@breza.me> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ofek Bashan <ofekray@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What did you implement:
Fixed mismatched ARM parameter names and spelling errors. Added interfaces for all resource ARM parameters to prevent future occurrence.
apimCapacity
to beapimSkuCapacity
storageAccoutTier
to bestorageAccountTier
How did you implement it:
getParameters
functionHow can we verify it:
No real functionality changes. Deploy should still work as normal
Todos:
Note: Run
npm run test:ci
to run all validation checks on proposed changesValidate via
npm test
Validate via
npm run lint
Note: Some reported issues can be automatically fixed by running
npm run lint:fix
Is this ready for review?: YES
Is it a breaking change?: NO