-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add a Lambda+Codepipeline Typescript example #269
Conversation
To the reviewer: regarding Conventional Commits, would you prefer if I squashed all the commits to something like:
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
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 excellent! My main concern is that some of the syntax choices were not consistent. Once thats fixed I'm happy to merge!
😸
) | ||
const editOrCreateLambdaDependencies = new PolicyStatement(); | ||
editOrCreateLambdaDependencies.addActions( | ||
"iam:GetRole", |
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.
"iam:GetRole", | |
"iam:GetRole", |
} | ||
}) | ||
|
||
const lambdaPolicy = new PolicyStatement(); |
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.
Please be consistent with semicolons. We recommend all lines terminate with semicolon, but if you want to follow the no-semicolon TS guidance, stick to that throughout the example.
@NGL321 Thanks for taking a look! I'll address that and update the pull request |
…hat might be breaking in the future
6149291
to
e158f16
Compare
@NGL321 The build seems to be failing with the following error:
|
This commit adds an example that deploys a Lambda+ApiGateway setup using CodePipeline. Although there's plenty of examples of Lambda already, deploying them via CodePipeline had some non-trivial challenges to sort out. This could mean teams start using CDK and like it, but drop it because they can't adopt Full CI/CD easily when using it
@@ -20,7 +20,7 @@ verify_star_dependencies() { | |||
} | |||
|
|||
# Find and build all NPM projects | |||
for pkgJson in $(find typescript -name package.json | grep -v node_modules); do | |||
for pkgJson in $(find typescript -name cdk.json | grep -v node_modules); do |
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.
I've run the following commands to compare the directories that are evaluated:
find typescript -name package.json | grep -v node_modules | xargs -n 1 dirname | sort
find typescript -name cdj.json | grep -v node_modules | xargs -n 1 dirname | sort
Here is the diff. Directories removed are shown in grey on the left-hand side:
Link
These directories are now excluded as they're Lambda NPM packages, not CDK packages:
typescript/lambda-api-ci/cdk.out/asset.5d8ebe4e1b7da78e3a974de62e8d9aac8a42747f5c303e249b6d1e2855c6e92f
typescript/lambda-api-ci/src
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.
Thank you for the catch! I had not noticed this. The fix is logical and LGTM.
😸
@NGL321 Looks like mergify discarded your approval after the rebase. Can you take another look? |
Well thats frustrating! Taking care of now! 😸 😷 |
@NGL321 I think mergify blew up again 🤦 |
I'm adding an example here that deploys a Lambda+ApiGateway setup using CodePipeline.
Although there's plenty of examples of Lambda already, deploying them via CodePipeline had some non-trivial challenges to sort out. This could mean teams start using CDK and like it, but drop it because they can't adopt Full CI/CD easily when using it
Fixes #267
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.