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.

Sign up for GitHub

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

Jump to bottom

initial implementation #2

Merged
merged 20 commits into from
Mar 17, 2022
Merged

initial implementation #2

merged 20 commits into from
Mar 17, 2022

Conversation

mcalhoun
Copy link
Member

@mcalhoun mcalhoun commented Feb 7, 2022 •
edited
Loading

what

  • Initial implementation of the module

@mcalhoun mcalhoun changed the title [WIP] initial implementation initial implementation Mar 16, 2022
@mcalhoun mcalhoun marked this pull request as ready for review March 16, 2022 18:35
@mcalhoun mcalhoun requested review from a team as code owners March 16, 2022 18:35
@mcalhoun mcalhoun requested review from Gowiem and dotCipher and removed request for a team March 16, 2022 18:35
@korenyoni
Copy link
Member

/test all

korenyoni
korenyoni approved these changes Mar 16, 2022
View reviewed changes
Copy link
Member

@korenyoni korenyoni left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Left comments but still LGTM for a first pass

examples/complete/main.tf
Comment on lines +9 to +13
data "archive_file" "lambda_zip" {
type = "zip"
source_file = "handler.js"
output_path = "lambda_function.zip"
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we delegate this to the root module?

See: https://github.com/cloudposse/terraform-aws-cloudfront-s3-cdn/tree/master/modules/lambda%40edge#lambdaedge

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we want to be prescriptive on how to create the zip file in this module. For instance, it could contain multiple files, dependencies (such as node_modules), etc. We should leave it up to the caller to determine how the zip file gets built.

examples/docker-image/main.tf Outdated Show resolved Hide resolved
iam-role.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@mcalhoun mcalhoun requested a review from a team as a code owner March 16, 2022 18:55
@mcalhoun
Copy link
Member Author

/test all

@mcalhoun
Copy link
Member Author

/test all

@mcalhoun
Copy link
Member Author

/test all

@mcalhoun
Copy link
Member Author

/test all

@mcalhoun
Copy link
Member Author

/test all

@mcalhoun mcalhoun merged commit 80a6d58 into main Mar 17, 2022
@mcalhoun mcalhoun deleted the feature/initial-implementation branch March 17, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@korenyoni korenyoni korenyoni approved these changes

@Gowiem Gowiem Awaiting requested review from Gowiem Gowiem is a code owner automatically assigned from cloudposse/contributors

@dotCipher dotCipher Awaiting requested review from dotCipher dotCipher is a code owner automatically assigned from cloudposse/contributors

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mcalhoun @korenyoni @cloudpossebot