chatterino2 icon indicating copy to clipboard operation
chatterino2 copied to clipboard

[CI] only run build workflows when required

Open jupjohn opened this issue 2 years ago • 7 comments

Describe your issue

Currently each push to a PR source branch causes the CI workflows to kick off. As great as it is knowing that your project is building, this process is incredibly slow and is a waste of minutes when:

A. a PR is marked as draft B. a PR isn't in a buildable state C. more changes are going to be pushed

Possible solutions:

  • Using a label to indicate the CI can be run (as @Felanbird suggested)
  • Use a comment-based trigger using a bot (done by Azure pipelines with the /azp run command)
  • Depend on approved code reviews to run pipelines

Checks would still need to be validated before a PR can be merged.

jupjohn avatar Mar 28 '22 22:03 jupjohn

@Felanbird bing, created to keep the discussion somewhere

jupjohn avatar Mar 28 '22 22:03 jupjohn

@jammehcow I looked into this and it seems the labeling concept is not officially supported. The approach Github was willing to take was using any of the following in your commit name/body: [skip ci], [ci skip], [no ci], [skip actions], or [actions skip].

This works exactly as expected and the workflows don't even attempt to fire. image

The user created workaround for this involves including this if: in your steps: ${{ !contains(github.event.pull_request.labels.*.name, 'asd123') }}

but I was unable to consistently get this to work (possibly due to my lack of ci knowledge), I just went with the hard approach of putting the if: step above every check, which works for all steps except the main build step, but sometimes it works on the main build step for macOS. https://github.com/Felanbird/chatterino2/actions/runs/2055489905 // macOS 5.12.10 qmake skipped through all checks as expected. https://github.com/Felanbird/chatterino2/actions/runs/2055554477 // macOS 5.12.2 qmake/cmake & 5.12.10 qmake all fail at startup, while macOS 5.12.10 cmake does not.

Also I did this with the changes to be implemented in #3631 || Felanbird/chatterino2#25

Felanbird avatar Mar 29 '22 00:03 Felanbird

can we just make a label called "ci approved"? Then it'll only run when that label is on the pr?

ALazyMeme avatar Mar 30 '22 07:03 ALazyMeme

I can't see why this is that big of an issue. I would hate to have to add another label to every pr I open for basic issues to be checked. If anything I'd prefer a ci skip label/commit tag.

Using code reviews for running ci misses the mark IMO, I tend to use ci as a basic indicator of usability, reviewing a pr that doesn't even compile isn't very useful.

Mm2PL avatar Mar 30 '22 08:03 Mm2PL

I dont get why this is even a problem, I always thought public repos had unlimited CI minutes, so why not use those for some more comfort when testing PRs?

LosFarmosCTL avatar Mar 30 '22 08:03 LosFarmosCTL

If anything I'd prefer a ci skip label/commit tag.

That's why my testing was the opposite of what was noted in the issue.

Using code reviews for running ci misses the mark IMO, I tend to use ci as a basic indicator of usability, reviewing a pr that doesn't even compile isn't very useful.

I originally prompted the idea because of chatterino7#75 - I believe it's completely reasonable to want to place the groundwork, but keep a formatting standard you might visually like better but not want to hog up the severs while a PR is a draft anyway. (but in that exact scenario the supported features of [no ci] would work better)

I dont get why this is even a problem, I always thought public repos had unlimited CI minutes, so why not use those for some more comfort when testing PRs?

General respect for only using what we need, just because something is unlimited doesn't mean we shouldn't be reasonable about its usage, we don't need to be hogging available servers just because. I get that general vibe from pajlada's comments about saving CI hours as well. 🤷

Felanbird avatar Mar 30 '22 10:03 Felanbird

The other reason I'd like this implemented is to reduce strain on self-hosted runners (if we use them). Wasting hours on self-hosted instances is much more of a problem and could cause builds to back up.

If my own runners get used I wouldn't be happy to have builds running on every single change. Currently waiting on qmake to be dropped as I'm unable to provide resources so won't impact me in the near future.

jupjohn avatar Apr 03 '22 06:04 jupjohn

I think our CI is good enough as-is right now, so I'll go ahead and close this one. If we start using self-hosted runs and have to be more sparing in our CI-hours, we can re-open this issue or create a new one with the issues we're having.

pajlada avatar Apr 06 '24 16:04 pajlada