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: cleanup into a single per-pr.yml gha #295

Merged
merged 6 commits into from
Feb 18, 2025
Merged

ci: cleanup into a single per-pr.yml gha #295

merged 6 commits into from
Feb 18, 2025

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Feb 18, 2025

Much needed cleanup of CI pipelines. We now have 2 main pipelines:

  • one that runs on every PR: runs tests, lint, builds
  • one that runs on every release: pushes image to GHCR

Only downside of this cleanup is that we can't have separate README badges for the different jobs (GHA only allows one badge for the entire workflow). Personally I prefer having a cleaner CI than extra precision of badges on the repo.

Even considering switching our CI to use https://docs.dagger.io/ at some point.

Fixes Issue

Fixes #

Changes proposed

Screenshots (Optional)

Note to reviewers

We will have to update the required workflows in the github repo settings. The old workflows that have been deleted (but are still required) will never pass. We should prob update them right before merging this PR. I can do that once this PR is approved.

litt3
litt3 previously approved these changes Feb 18, 2025
Copy link
Collaborator

@litt3 litt3 left a comment

Choose a reason for hiding this comment

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

LGTM

@ethenotethan
Copy link
Collaborator

not a big fan of this change tbh. would prefer to have separate workflows given differing checks vs restructuring the file schema to be workflows specific to dispatch conditions. IMO restructuring or consolidating would make sense with workflows that have overlapping dependencies (e.g, linting and testing both use a golang job for initializing the language dependency in the runner's environment). Also have never seen this pattern before (e.g, in the eigenda repo we delineate workflows based on higher order checks). Anyways if this is smth we'd like to adopt I'd prefer at a minimum we change the workflow file name from per-pr.yml to something like action.yml

ethenotethan
ethenotethan previously approved these changes Feb 18, 2025
Copy link
Collaborator

@ethenotethan ethenotethan left a comment

Choose a reason for hiding this comment

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

LGTM

@samlaf samlaf dismissed stale reviews from ethenotethan and litt3 via c49c370 February 18, 2025 21:04
Copy link
Collaborator

@ethenotethan ethenotethan left a comment

Choose a reason for hiding this comment

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

LGTM

@samlaf samlaf merged commit d7b1642 into main Feb 18, 2025
9 checks passed
@samlaf samlaf deleted the ci--cleanup branch February 18, 2025 21:11
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