-
Notifications
You must be signed in to change notification settings - Fork 825
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
fix: stops cdk v1 warning for non cdk custom resource #12241
Conversation
@@ -65,7 +65,7 @@ const getOverridesWarning = (resourcesToBuild: IAmplifyResource[], dependencyToS | |||
const getCustomResourcesWarning = (resourcesToBuild: IAmplifyResource[], dependencyToSearch: string): AmplifyWarning | undefined => { | |||
let customResourcesWarningObject; | |||
const customResourceImpactedFiles = []; | |||
const customCategoryResources = resourcesToBuild.filter((resource) => resource.category === AmplifyCategories.CUSTOM); | |||
const customCategoryResources = resourcesToBuild.filter((resource) => resource.category === AmplifyCategories.CUSTOM && resource.service !== 'customCloudformation'); |
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.
consider extracting second part of condition to !isCFNBasedCustomeResource(resource)
for future readers.
await addCFNCustomResource(projRoot, { name: cfnResourceNameWithV10, promptForCategorySelection: false }); | ||
const srcCFNCustomResourceFilePath = path.join(__dirname, '..', '..', '..', 'custom-resources', 'custom-cfn-stack.json'); | ||
// adding a resource to custom cfn stack | ||
const destCFNCustomResourceFilePath = path.join(projRoot, 'amplify', 'backend', 'custom', cfnResourceNameWithV10,`${cfnResourceNameWithV10}-cloudformation-template.json`); | ||
fs.copyFileSync(srcCFNCustomResourceFilePath, destCFNCustomResourceFilePath); |
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.
consider adding one non-migration test that would spin up only CFN based custom without any CDK.
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## dev #12241 +/- ##
==========================================
- Coverage 49.45% 47.15% -2.30%
==========================================
Files 692 836 +144
Lines 33424 38038 +4614
Branches 6840 7715 +875
==========================================
+ Hits 16529 17937 +1408
- Misses 15370 18465 +3095
- Partials 1525 1636 +111
... and 456 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
) * fix: stops cdk warning for non cdk custom resource * fix: added unit test --------- Co-authored-by: Akshay Upadhyay <[email protected]>
* fix: stops cdk warning for non cdk custom resource * fix: added unit test --------- Co-authored-by: akshbhu <[email protected]> Co-authored-by: Akshay Upadhyay <[email protected]>
Description of changes
Issue #, if available
Description of how you validated changes
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.