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

chore(ci): enable experimental for the PR image builds #1976

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

vpavlin
Copy link
Member

@vpavlin vpavlin commented Aug 31, 2023

Description

Spliting out the container image build into a separate workflow and invoking it from both ci and ci-experimental. This results in 2 images being pushed - ${PR_NUMBER} and ${PR_NUMBER}-experimental - thought it might be useful with regards to the push on RLN lately

Changes

  • split out container-image.yml workflow
  • execute it from both ci and ci-experimental workflows

Comment on lines +43 to +45
EXPERIMENTAL=${{ inputs.experimental }}

make -j${NPROC} V=1 QUICK_AND_DIRTY_COMPILER=1 NIMFLAGS="-d:disableMarchNative" wakunode2 EXPERIMENTAL=${{EXPERIMENTAL}}
Copy link
Contributor

Choose a reason for hiding this comment

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

curious if we can have this workflow depend on the build job in the ci workflow

we can probably skip the docker image building if the ci workflow fails, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could, my thinking initially was to provide the image ASAP rather than wait for the CI to finish, but we definitely can change that

Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

LGTM

@vpavlin vpavlin force-pushed the chore/pr-image-build-improvement branch from c26560d to c71db4a Compare September 1, 2023 07:01
Copy link
Collaborator

@Ivansete-status Ivansete-status left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for it!


build-docker-image:
needs: changes
if: ${{ needs.changes.outputs.v2 == 'true' || needs.changes.outputs.common == 'true' || needs.changes.outputs.docker == 'true' }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is is needed to add the next snippet in the "filter change detection" above?

          docker:
          - 'docker/**'

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes, I added that for the ci, but not for the ci-experimental, good catch

@vpavlin vpavlin force-pushed the chore/pr-image-build-improvement branch from c71db4a to f61e03e Compare September 4, 2023 07:37
@vpavlin vpavlin merged commit 1b835b4 into master Sep 4, 2023
2 checks passed
@vpavlin vpavlin deleted the chore/pr-image-build-improvement branch September 4, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants