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

Use artifacts for docker images #108

Merged
merged 49 commits into from
Jun 15, 2023

Conversation

EmilyBourne
Copy link
Collaborator

@EmilyBourne EmilyBourne commented Dec 12, 2022

Use artifacts for docker images to ensure forks can contribute.

.github/workflows/pages.yml Outdated Show resolved Hide resolved
jbigot
jbigot previously approved these changes Mar 16, 2023
.github/workflows/pages.yml Show resolved Hide resolved
jbigot
jbigot previously approved these changes Mar 16, 2023
Copy link
Member

@jbigot jbigot left a comment

Choose a reason for hiding this comment

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

perfect, thanks !

@tpadioleau
Copy link
Member

Shall it be merged ?

@jbigot
Copy link
Member

jbigot commented Apr 20, 2023

Shall it be merged ?

not as it is, it will make tests that didn't fail before break because of space issue. As mentioned earlier, I believe this is a nice addition, but it should be used as a fallback only when pushing from a fork:

I don't see a good solution that will work all the time.

My proposition is to introduce a condition in the code to test from which repository the source branch is:

when on the original repository, use the initial solution using docker push,
when on a different directory, use the version with artifact.

That way, we shouldn't have random failure in case that already worked, but new cases when pulling from external repo might work (maybe after a few tries due to size limitations)

@tpadioleau tpadioleau marked this pull request as draft April 27, 2023 12:50
@EmilyBourne EmilyBourne marked this pull request as ready for review April 28, 2023 08:35
@jbigot
Copy link
Member

jbigot commented Jun 15, 2023

Alrighty ! Thank you very much, @EmilyBourne !

@jbigot jbigot merged commit e2f37cf into CExA-project:main Jun 15, 2023
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.

Forked branches can't create pull requests due to docker handling
3 participants