-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(codepipeline): variables for CodeStar Connections source Action #18086
Conversation
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
69a37ae
to
891c647
Compare
@skinny85 no idea what I am doing wrong with semantic pull request. I think it follows the guideline but apparently I am missing an important detail |
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 @kornicameister! A few tiny comments before we merge this in, but this is in great shape already.
packages/@aws-cdk/aws-codepipeline-actions/lib/codestar-connections/source-action.ts
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
On holiday now. I will try to get back to it after Xmas but I cannot promise that. |
Pull request has been modified.
Adds `variables` method that allows to retrieve action's [variables](https://docs.aws.amazon.com/codepipeline/latest/userguide/reference-variables.html) using type safe approach. With that we can rely on compiler to pick up incorrect access instead of hacking it with plain strings. fixes: aws#17807
14a3f9e
to
78c70d4
Compare
@skinny85 ready for another round. |
Looks like the linter is failing the build:
|
Yeah, it wasn't picked by |
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 @kornicameister! A few suggestions on the test, that I will address myself.
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
...ws-codepipeline-actions/test/codestar-connections/codestar-connections-source-action.test.ts
Outdated
Show resolved
Hide resolved
Pull request has been modified.
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.
Thanks for the contribution @kornicameister!
Pull request has been modified.
Pull request has been modified.
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). |
…ws#18086) Adds `variables` method that allows to retrieve action's [variables](https://docs.aws.amazon.com/codepipeline/latest/userguide/reference-variables.html) using type safe approach. With that we can rely on compiler to pick up incorrect access instead of hacking it with plain strings. fixes: aws#17807 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Adds
variables
method that allows to retrieve action's variablesusing type safe approach.
With that we can rely on compiler to pick up incorrect access instead
of hacking it with plain strings.
fixes: #17807
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license