-
Notifications
You must be signed in to change notification settings - Fork 35
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
Publish preview images, add checks for the multi-arch image variants #120
Conversation
architectures.txt
Outdated
@@ -0,0 +1,8 @@ | |||
"386_linux_" |
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.
I think we should document this file at some place, like a RELEASING.md file or similar. Wdyt?
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.
Yes, that's a good idea. I could also rename the file to be more descriptive?
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.
"supported-architectures.txt"?
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.
Good and descriptive name 👍
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.
I've updated the filename and added minimal documentation.
As a thought: the workflows introduced here might be re-usable for other repositories in the "testcontainers" organisation. GitHub allows shared workflows. I would like to first copy the workflows from here to one or two other repositories (see the pull request in the helloworld repo), so that we're able to decide whether an abstraction with shared workflows makes sense and how it should look like. |
@mdelapenya did you have a chance to make a final review of the pull request? I would like to take the changes from here to the other pull request in the helloworld repository, so that we can continue working on WCOW support in the testcontainers-java repository. |
FYI @gesellix please be aware of the following limitations: testcontainers/testcontainers-dotnet#1140 (comment). |
Thanks for the heads up. I think we can still continue working on WCOW support, because it's an improvement of the status quo without breaking existing use cases. Whether Testcontainers will declare the WCOW support as official or fully supported feature is probably a decision when all the building blocks are in place. |
How and when are we going to continue here? I would like to improve the other tools, for example testcontainers/helloworld#5, aiming at testcontainers/testcontainers-java#8436 to become a step forward. Like already mentioned I would try to have some consistency across the different repositories (trying to minimize maintenance efforts), so this pull request could be a template for other repos. Consolidating them to a shared workflow would come after knowing all required variants. If this is something you don't want to continue, feel free to leave a note. I guess WCOW support doesn't have a high priority. It would be great, though, if there would be some kind clarity in either working on WCOW support or explicitly not doing 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.
LGTM thanks!
@gesellix thanks for your work here, as always. It was delayed for no reason, so please forgive me if it took that long. |
@gesellix the diff failed in the merge commit. Could you take a look please? |
No reason to ask for forgiveness, my previous comment might look a bit harsh, but wasn't meant like that. I'm totally aware that maintaining many different projects is a lot of work and that sometimes less important aspects get out of sight :) |
Yes, I'll try to investigate asap (this week)! |
@mdelapenya I found two issues, both have been fixed at #130 (see the details over there). |
Follow up of #110