-
Notifications
You must be signed in to change notification settings - Fork 180
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
Fix issue created by download_sigil.sh #4343
Conversation
19d1b3e
to
dc81dc7
Compare
Any issues with this PR @bjester? Wanna make sure unstable still isn't broken 😅 |
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.
Github action looks like it needs a small update
.github/workflows/nginxtest.yml
Outdated
runs-on: ubuntu-latest | ||
# Map a step output to a job output | ||
outputs: | ||
should_skip: ${{ steps.skip_check.outputs.should_skip }} |
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.
This doesn't appear to be doing anything with should_skip
and hence skipping the build if unnecessary
@aronasorman I created a new workflow that we needed for building the postgres image with pg-vector. Let me know if you would like me to consolidate yours here with it https://github.com/learningequality/studio/blob/unstable/.github/workflows/containerbuild.yml |
Yup works for me! I made a separate file because i want to write more tests in the future. But I don't know when I'll actually get to that. Happy to merge it to your new pg file if it makes more sense. |
Nginx isn't fully integrated yet. We add it here so we can run a docker build and test that it's working
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.
Combined container builds into a single workflow.
From #4336 (comment), We use the wrong arch format (x86-64 vs. amd64, the latter is the one used by the Sigil project).
Also, error out sooner by using
set -e
on the script preamble.Update: We now also build the Nginx docker image during PRs and one of the critical branches.