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: codebuild ci workflow for external contributors #1207

Merged
merged 11 commits into from
Feb 19, 2024

Conversation

0marperez
Copy link
Contributor

Issue #

N/A

Description of changes

Added inputs to workflow. Inputs will be passed along to the codebuild job via env var

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

@0marperez 0marperez added the no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly. label Feb 8, 2024
Copy link

github-actions bot commented Feb 8, 2024

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@0marperez 0marperez marked this pull request as ready for review February 8, 2024 21:28
@0marperez 0marperez requested a review from a team as a code owner February 8, 2024 21:28
@@ -29,6 +47,7 @@ jobs:
uses: aws-actions/aws-codebuild-run-build@v1
with:
project-name: gh-aws-sdk-kotlin-e2e-tests
env-vars-for-codebuild: SDK_PR, SMITHY_PR
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Why doesn't the service check batch job require similar changes? Why does it work (or does it)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to add similar changes to service-check-batch still

type: number
required: false
check-pr:
description: I verified that the PR's are not running any malicious code
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: PR's (possessive) → PRs (plural)

Comment on lines 17 to 21
check-pr:
description: I verified that the PR's are not running any malicious code
required: true
type: boolean
default: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: What does this setting actually do? I don't see it used anywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's meant as a reminder, it can be enforced though. The workflow could not run if the checkbox isn't marked

Copy link
Contributor

Choose a reason for hiding this comment

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

I think enforcement is a good idea. A checkbox that has no effect and can be left unchecked with no repercussions seems pointless.

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@@ -38,21 +59,31 @@ jobs:
echo "cancelling in-progress build: id=$BUILD_ID"
aws codebuild stop-build --id $BUILD_ID
fi
- name: Verify PRs are not running malicious code
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Move this as the first step and you won't need the other if checks on each step. A failure of an earlier step will prevent it from running.

@@ -62,3 +93,8 @@ jobs:
echo "cancelling in-progress batch build: id=$BUILD_ID"
aws codebuild stop-build --id $BUILD_ID
fi
- name: Verify PRs are not running malicious code
Copy link
Contributor

Choose a reason for hiding this comment

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

Same suggestion, move to earlier and you won't need a conditional on every step

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK

@0marperez 0marperez merged commit 2ee1a34 into main Feb 19, 2024
16 checks passed
@0marperez 0marperez deleted the codebuild-ci-workflow branch February 19, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a changelog entry isn't required for a pull request. Use sparingly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants