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

feat(lambda): allow inline code for nodejs12.x runtime #5710

Merged
merged 6 commits into from Jan 16, 2020
Merged

feat(lambda): allow inline code for nodejs12.x runtime #5710

merged 6 commits into from Jan 16, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jan 8, 2020 •
edited by ghost
Loading

The nodejs12.x Lambda runtime actually support inline code.

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

chore(lambda): new runtimes support inline code
9523687 
The nodejs12.x and python3.8 Lambda runtimes actually support inline code.

This reverts commit feb1f9b.
@ghost ghost requested review from eladb and nija-at as code owners January 8, 2020 13:25
@ghost ghost changed the title chore(lambda): new runtimes support inline code chore(lambda): new nodejs12.x runtime supports inline code Jan 8, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

nija-at
nija-at previously requested changes Jan 9, 2020
View reviewed changes
Copy link
Contributor

@nija-at nija-at 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.

Can you describe how you tested this?

Update: Let's add an integration test that contain multiple lambda functions, one for every runtime that supports inline code. Make sure you're able to deploy this stack into your AWS account.

@eladb eladb changed the title chore(lambda): new nodejs12.x runtime supports inline code feat(lambda): nodejs12.x runtime now supports inline code Jan 12, 2020
eladb
eladb previously approved these changes Jan 12, 2020
View reviewed changes
@eladb
Copy link
Contributor

eladb commented Jan 12, 2020

Update: Let's add an integration test that contain multiple lambda functions, one for every runtime that supports inline code. Make sure you're able to deploy this stack into your AWS account.

I feel this might be a bit of an overkill... Maybe @gruenem can just note in their PR description how they tested this.

eladb
eladb reviewed Jan 12, 2020
View reviewed changes
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

Revoking approval seeing @nija-at's comment.

@nija-at
Copy link
Contributor

nija-at commented Jan 13, 2020

Update: Let's add an integration test that contain multiple lambda functions, one for every runtime that supports inline code. Make sure you're able to deploy this stack into your AWS account.

I feel this might be a bit of an overkill... Maybe @gruenem can just note in their PR description how they tested this.

Sure, that's a start.
However, I don't think this is overkill. Having an integration test in our codebase and calling out in the author stating in the PR description that the integration test was run is a good verification of such a change. It removes the ambiguity to the details of how it was tested.

@nija-at nija-at mentioned this pull request Jan 14, 2020
Merged
@nija-at
Copy link
Contributor

nija-at commented Jan 16, 2020

I've gone ahead and added said integration test.

@mergify mergify bot dismissed stale reviews from nija-at and eladb January 16, 2020 12:05

Pull request has been modified.

@nija-at nija-at changed the title feat(lambda): nodejs12.x runtime now supports inline code feat(lambda): allow inline code for nodejs12.x runtime Jan 16, 2020
nija-at
nija-at previously approved these changes Jan 16, 2020
View reviewed changes
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot dismissed nija-at’s stale review January 16, 2020 12:57

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jan 16, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jan 16, 2020

Thank you for contributing! Your pull request is now being automatically merged.

@mergify mergify bot merged commit a1cd743 into aws:master Jan 16, 2020
@ghost ghost deleted the patch-1 branch January 16, 2020 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nija-at nija-at

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aws-cdk-automation @eladb @nija-at