-
Notifications
You must be signed in to change notification settings - Fork 55
feat: release readiness check #1560
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
Conversation
|
A new generated diff is ready to view.
|
This comment has been minimized.
This comment has been minimized.
| on: | ||
| pull_request: | ||
| branches: [ main ] | ||
| workflow_dispatch: |
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.
question: What is the workflow_dispatch needed for?
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.
It allows the workflow to be triggered manually. I don't know if we'll necessarily need to but seems fine to me.
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.
Yeah, that's where I was leading with the question. I don't think we'll ever need to trigger this workflow manually, so we could simplify this a little by removing it?
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.
Sure, we can remove it and if we ever need to run it manually we can add the workflow_dispatch back
| on: | ||
| pull_request: | ||
| branches: [ main ] | ||
| workflow_dispatch: |
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.
It allows the workflow to be triggered manually. I don't know if we'll necessarily need to but seems fine to me.
| - name: Configure error message | ||
| run: echo "ERROR_MESSAGE=WARNING smithy-kotlin release and version bump might be required before merging!" >> $GITHUB_ENV | ||
|
|
||
| - name: Build SDK | ||
| run: ./gradlew test jvmTest || echo $ERROR_MESSAGE | ||
|
|
||
| - name: Build SDK client | ||
| run: | | ||
| ./gradlew -Paws.services=s3 -Paws.kotlin.native=false bootstrap || echo $ERROR_MESSAGE | ||
| ./gradlew build || echo $ERROR_MESSAGE No newline at end of file |
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.
Question: How does this work in the failure case?
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.
If the command exits with a non-zero exit code then we'll echo the message at the end of the output
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.
But echo returns a zero exit code, so does the CI properly report a failed status?
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.
Oh I see what you mean, it wouldn't. Fixed
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
|
A new generated diff is ready to view. |
This comment has been minimized.
This comment has been minimized.
| - name: Configure error message | ||
| run: echo "ERROR_MESSAGE=WARNING smithy-kotlin release and version bump might be required before merging" >> $GITHUB_ENV | ||
|
|
||
| - name: Build SDK | ||
| run: ./gradlew test jvmTest || { echo "$ERROR_MESSAGE"; exit 1; } | ||
|
|
||
| - name: Build SDK client | ||
| run: | | ||
| ./gradlew -Paws.services=s3 -Paws.kotlin.native=false bootstrap || { echo "$ERROR_MESSAGE"; exit 1; } | ||
| ./gradlew -Paws.kotlin.native=false build || { echo "$ERROR_MESSAGE"; exit 1; } |
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.
Nit: I think this is still more complicated than it needs to be. A step can be configured to run on failure so that a GitHub-standard error message can be emitted:
jobs:
release-readiness:
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v4
- name: Configure Gradle
uses: awslabs/aws-kotlin-repo-tools/.github/actions/configure-gradle@main
- name: Build SDK
run: ./gradlew test jvmTest
- name: Build SDK client
run: |
./gradlew -Paws.services=s3 -Paws.kotlin.native=false bootstrap
./gradlew -Paws.kotlin.native=false build
- name: Emit error message
if: ${{ failure() }}
run: |
echo "::error ::Build failed. Did you forget to release smithy-kotlin and bump the dependency version?"
exit 1|
A new generated diff is ready to view. |
… release-readiness
|
|
A new generated diff is ready to view. |
Affected ArtifactsNo artifacts changed size |



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