-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(cli): hotswap for appsync vtl mapping template changes #18881
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
Conversation
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.
It's not often that an Internet celebrity submits a Pull Request to your project, and even rarer when the celebrity writes such excellent code! 😉
This is a great contribution that I'm sure a lot of AppSync users will really appreciate. I have a few comments, but they're relatively minor.
Thanks for your work on this @adamelmore!
packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
Show resolved
Hide resolved
I got a good chuckle out of "internet celebrity" and "excellent code" 😅. Thanks for this, Adam, I'll hop on these changes early AM tomorrow! |
Pull request has been modified.
Alright @skinny85, I've made the requested changes, I think! Things shifted quite a bit (hopefully for the better), so hit me with another round when you have the time! |
9579931
to
1fcc06c
Compare
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 looks great @adamelmore, much shorter than the separate version!
I have a few last comments, but they're all minor, and in the next iteration we'll merge this in!
packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
Outdated
Show resolved
Hide resolved
Pull request has been modified.
Alright, ready for your 👀 again @skinny85! |
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 great @adamelmore, thanks for the contribution!
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
I do a lot of updating VTL mapping templates for my AppSync API throughout the day, and I'd love to have those changes hot swapped through the SDK for faster feedback loops. This PR handles changes to `RequestMappingTemplate` and `ResponseMappingTemplate` for both resolvers (`AWS::AppSync::Resolver`) and functions (`AWS::AppSync::FunctionConfiguration`). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
I do a lot of updating VTL mapping templates for my AppSync API throughout the day, and I'd love to have those changes hot swapped through the SDK for faster feedback loops.
This PR handles changes to
RequestMappingTemplate
andResponseMappingTemplate
for both resolvers (AWS::AppSync::Resolver
) and functions (AWS::AppSync::FunctionConfiguration
).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license