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

CI bug for main/release #211

Merged
merged 9 commits into from
Jul 17, 2023
Merged

CI bug for main/release #211

merged 9 commits into from
Jul 17, 2023

Conversation

cbrit
Copy link
Member

@cbrit cbrit commented Jun 28, 2023

No description provided.

Copy link
Contributor

@EricBolten EricBolten left a comment

Choose a reason for hiding this comment

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

A question around Docker usage.

- name: Make upload dir
run: mkdir -p ${{ env.UPLOAD_DIR }}
- name: build-and-push
uses: docker/build-push-action@v2
with:
file: Dockerfile
push: false
tags: steward:prebuilt
push: ${{ github.event_name != 'pull_request' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose my main question here is: do we need to push to Docker at all for steward and hardhat images in our CI workflows? Couldn't we just build the steward and hardhat docker images as artifacts and remove the push aspect entirely?

Copy link
Member Author

@cbrit cbrit Jun 30, 2023

Choose a reason for hiding this comment

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

my thinking was we wanted the steward:main tag to get pushed when merging to main?

afa hardhat, maybe not so much

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't really need to publish any docker images unless something else needs to pull them. Steward as a repo is effectively a leaf node in the dependency tree.

@cbrit cbrit temporarily deployed to CI July 11, 2023 01:35 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 11, 2023 01:35 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 11, 2023 01:35 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 11, 2023 01:35 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:01 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:01 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:01 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:01 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:37 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:37 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:37 — with GitHub Actions Inactive
@cbrit cbrit temporarily deployed to CI July 13, 2023 22:37 — with GitHub Actions Inactive
Copy link
Contributor

@EricBolten EricBolten left a comment

Choose a reason for hiding this comment

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

LGTM

@EricBolten EricBolten merged commit 0ef6979 into main Jul 17, 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.

2 participants