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

fix(amplify-category-function): use ref for S3Bucket and S3Key in CFN #6358

Merged
merged 5 commits into from Jan 22, 2021

Conversation

jhockett
Copy link
Contributor

Issue #, if available:
Closes #5387

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@lgtm-com
Copy link

lgtm-com bot commented Jan 11, 2021

This pull request introduces 3 alerts when merging 3f0c9b0 into 7a045fb - view on LGTM.com

new alerts:

  • 3 for Superfluous trailing arguments

@codecov
Copy link

codecov bot commented Jan 11, 2021

Codecov Report

Merging #6358 (0624bd3) into master (b5f917a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #6358    +/-   ##
========================================
  Coverage   57.21%   57.21%            
========================================
  Files         474      474            
  Lines       21609    21609            
  Branches     4078     4295   +217     
========================================
  Hits        12364    12364            
+ Misses       8384     8364    -20     
- Partials      861      881    +20     
Impacted Files Coverage Δ
packages/amplify-cli-core/src/index.ts 94.73% <ø> (ø)
packages/amplify-util-mock/src/api/api.ts 0.00% <0.00%> (ø)
packages/graphql-mapping-template/src/print.ts 35.29% <0.00%> (ø)
packages/amplify-util-mock/src/storage/storage.ts 0.00% <0.00%> (ø)
...ges/amplify-util-mock/src/CFNParser/stack/index.ts 0.00% <0.00%> (ø)
...es/amplify-util-mock/src/api/resolver-overrides.ts 0.00% <0.00%> (ø)
...es/graphql-transformer-core/src/stripDirectives.ts 35.29% <0.00%> (ø)
.../amplify-cli-core/src/state-manager/pathManager.ts 67.44% <0.00%> (ø)
.../amplify-util-mock/src/utils/lambda/loadMinimal.ts 0.00% <0.00%> (ø)
...graphql-transformer-core/src/TransformerContext.ts 25.09% <0.00%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5f917a...0624bd3. Read the comment docs.

@jhockett jhockett marked this pull request as ready for review January 11, 2021 20:05
@jhockett jhockett requested a review from a team as a code owner January 11, 2021 20:05
Copy link
Member

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

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

just one question, but LGTM

@r0zar
Copy link
Contributor

r0zar commented Feb 5, 2021

👏 🎉 thanks @jhockett

@Nxtra
Copy link

Nxtra commented Feb 12, 2021

Does this mean I have to manually update my existing functions?

@jhockett
Copy link
Contributor Author

Does this mean I have to manually update my existing functions?

No, it should be taken care of automatically on the next amplify push. Did you run into any issues?

@Nxtra
Copy link

Nxtra commented Feb 15, 2021

Does this mean I have to manually update my existing functions?

No, it should be taken care of automatically on the next amplify push. Did you run into any issues?

My existing functions weren't updated. But I now see, that you have to touch the function. Eg adding a console.log, there by having the function recognized as updated, will trigger the changes in the cloudformation template.

@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 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.

Stop storing S3 bucket Lambda deployment information under Git
4 participants