-
Notifications
You must be signed in to change notification settings - Fork 30
ci: add Docker build workflow for PR checks #385
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
ci: add Docker build workflow for PR checks #385
Conversation
Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerOriginal prompt from [email protected]: I'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
I don't think an explicit check push is required. We may get the same value already by the default status handling. Devin, can you confirm? |
…ns status handling Co-Authored-By: [email protected] <[email protected]>
|
Hi Aaron, you're right! I've confirmed that explicit check push is not required. GitHub Actions automatically creates status checks for workflow runs without explicit API calls. I've removed the explicit GitHub status API calls and am now relying on default GitHub Actions status handling. I've also renamed the job to 'SDM Docker Image Build' to provide a more descriptive status check name. The main difference between the explicit and default status handling is that with default handling, we lose the ability to have custom descriptions like 'SDM Docker image builds successfully' or 'SDM Docker image is broken'. Instead, GitHub uses its default descriptions like 'Success' or 'Failure'. However, the status check still clearly indicates whether the Docker build succeeded or failed. You can see the changes in the latest commit: 654bce2 |
|
Hi @natikgadzhi, thanks for your comment! The workflow is now set up to do exactly that - it will show a success status if the Docker image builds successfully, and a failure status if it errors out. With the changes I've made to remove the explicit status API calls and rely on default GitHub Actions status handling, the workflow will still clearly indicate whether the build succeeded or failed, which is exactly what you need. The job is now named 'SDM Docker Image Build' to make it clear what's being tested. |
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.
To my eyes, this looks good. One question inline about perhaps running this test on all commits, versus just on main. I don't feel strongly either way, and we can change it later if we want.
Sidebar: I have a similar workflow in the pypi_publish.yml workflow - which itself is a known misnomer due to needing to keep the file name aligned with what PyPi expects the file to be named. I checked that flow and confirmed these are not redundant, since those build steps only run on releases or prereleases.
We can add here and then later consider consolidating with the other file.
This PR adds a GitHub workflow that runs on pull requests against the main branch and attempts to build the Docker image in the root of the repository for linux/arm64 and linux/amd64 platforms. If the build succeeds, it posts a check on the PR indicating that the SDM Docker Image builds successfully. If the build fails, it posts a check indicating that the SDM Docker image is broken.
Link to Devin run: https://app.devin.ai/sessions/390a513e6e164d06a106d0cfca591a2f
Requested by: user in Slack channel #dev-devin-ai