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

feat(aws-codebuild): allow github sourceversion branch #5890

Merged
merged 9 commits into from
Feb 4, 2020
Merged

feat(aws-codebuild): allow github sourceversion branch #5890

merged 9 commits into from
Feb 4, 2020

Conversation

knorms101
Copy link
Contributor

Addresses issue #5777.

Allows users to specify which GitHub branch a CodeBuild should use as its Source Version. Currently the branch always defaults to 'master', but users may want to test features on other branches


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

Addresses issue #5777.Users may want to specify which github branch a codebuild should use as its source. Currently the branch always defaults to 'master', but users may want to test features in development on other branches
@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

@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

Copy link
Contributor

@skinny85 skinny85 left a 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 @knorms101 , it's great! I have a few small comments.

From looking at the docs of this feature in http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-codebuild-project.html#cfn-codebuild-project-sourceversion , I see this also works for CodeCommit, BitBucket and S3. Do you have time to add those as well? (S3 probably should be called version instead of branch).

Thanks!

packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
@knorms101
Copy link
Contributor Author

Thanks for the contribution @knorms101 , it's great! I have a few small comments.

From looking at the docs of this feature in http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-codebuild-project.html#cfn-codebuild-project-sourceversion , I see this also works for CodeCommit, BitBucket and S3. Do you have time to add those as well? (S3 probably should be called version instead of branch).

Thanks!

Thanks for the feedback, @skinny85! I agree that it would be great to include this option for the other CodeBuild sources too. I'll work on adding that next week.

@skinny85
Copy link
Contributor

Thanks for the contribution @knorms101 , it's great! I have a few small comments.
From looking at the docs of this feature in http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-codebuild-project.html#cfn-codebuild-project-sourceversion , I see this also works for CodeCommit, BitBucket and S3. Do you have time to add those as well? (S3 probably should be called version instead of branch).
Thanks!

Thanks for the feedback, @skinny85! I agree that it would be great to include this option for the other CodeBuild sources too. I'll work on adding that next week.

Awesome, thanks so much for taking the time to work on this! ❤️

@mergify mergify bot dismissed skinny85’s stale review January 30, 2020 19:04

Pull request has been modified.

@knorms101
Copy link
Contributor Author

Thanks for the contribution @knorms101 , it's great! I have a few small comments.
From looking at the docs of this feature in http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-codebuild-project.html#cfn-codebuild-project-sourceversion , I see this also works for CodeCommit, BitBucket and S3. Do you have time to add those as well? (S3 probably should be called version instead of branch).
Thanks!

Thanks for the feedback, @skinny85! I agree that it would be great to include this option for the other CodeBuild sources too. I'll work on adding that next week.

Awesome, thanks so much for taking the time to work on this! ❤️

Hi @skinny85 I finally had a chance to continue working on this. I added the optional sourceVersion property to all the other CodeBuild source options. I also added the optional secondarySourceVersions property.

@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

Copy link
Contributor

@skinny85 skinny85 left a 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 @knorms101 !

We're getting closer, but there are a few things that need changing still.

Let me know what you think of the name branchOrRef for the Git-based sources!

packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
…unused property. only render secondary sources versions when present.
@mergify mergify bot dismissed skinny85’s stale review February 2, 2020 16:14

Pull request has been modified.

@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

Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

This is fantastic @knorms101 ! If you could get rid of a tiny piece of duplication, it will be perfect 😃

packages/@aws-cdk/aws-codebuild/lib/source.ts Outdated Show resolved Hide resolved
@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

Copy link
Contributor

@skinny85 skinny85 left a 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 @knorms101 !

@mergify
Copy link
Contributor

mergify bot commented Feb 4, 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 Feb 4, 2020

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

@mergify mergify bot merged commit 155b80e into aws:master Feb 4, 2020
@knorms101 knorms101 deleted the knorms/codebuild-sourceversion-github-branch branch February 4, 2020 18:22
@skinny85 skinny85 mentioned this pull request Feb 6, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants