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

don't run GHA workflows on tag pushes #6510

Merged
merged 1 commit into from
Sep 4, 2024
Merged

don't run GHA workflows on tag pushes #6510

merged 1 commit into from
Sep 4, 2024

Conversation

iliana
Copy link
Contributor

@iliana iliana commented Sep 3, 2024

Per the branching experiment review we are about to begin in the control plane huddle.

  1. For the rust.yml workflow, only run the push mode when pushing to main or any branch matching the glob rel/**.
  2. For the hakari.yml workflow, apply the same rules. This has the effect of enabling them for the release branches, and also enabling them for pull requests that do not target the main branch.

This is a draft because we cannot merge it until Rust / clippy-lint (push) and Rust / check-style (push) are removed from the required checks. (The pull_request versions should remain required.) EDIT: Maybe this isn't a thing? GitHub confuses me.

@iliana iliana requested review from sunshowers and jclulow September 3, 2024 17:26
@iliana iliana marked this pull request as ready for review September 3, 2024 17:30
@jclulow
Copy link
Collaborator

jclulow commented Sep 3, 2024

GitHub confuses me.

The purpose of a system

on: [ push, pull_request ]
on:
push:
branches:
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if I understand correctly, the reason this is required is because GitHub ridiculously includes tags in the push event, and we had no filter at all here to begin with, but now we explicitly restrict it to this set of branches?

Do we want to, perhaps, do it for all branches (just not tags)? That's how buildomat works for example.

Copy link
Contributor Author

@iliana iliana Sep 3, 2024

Choose a reason for hiding this comment

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

I'm fine either way; I think the vast majority of push events to other branches are redundant with the pull request events, and we need the pull request events to handle PRs from forks.

Another approach might be to run the workflow on any push to a branch, and only run pull request workflows if they come from a fork. I don't know if that's possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In practice I think we would only particularly care if an artefact we expect to download and run (for a non-PR work in progress branch) was missing. Those come from buildomat, not GHA, so I expect making this change is likely fine as-is!

Copy link
Contributor

Choose a reason for hiding this comment

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

(have you had a chance to run the numbers on cost/time savings if buildomat did filter out non-PR branches, btw?)

Copy link
Contributor

@sunshowers sunshowers left a comment

Choose a reason for hiding this comment

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

thanks!

on: [ push, pull_request ]
on:
push:
branches:
Copy link
Contributor

Choose a reason for hiding this comment

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

(have you had a chance to run the numbers on cost/time savings if buildomat did filter out non-PR branches, btw?)

@iliana iliana merged commit 6bd835b into main Sep 4, 2024
16 checks passed
@iliana iliana deleted the iliana/no-tag-pushes branch September 4, 2024 00:48
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