Skip to content

CI: Ensure draft PRs don't perform actions #109407

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Aug 7, 2025

Tweaks the runner conditional such that a drafted PR will not perform actions. While I don't think it can be completely exempted from attempting to start an action, this should ultimately aid in cleaning up the action queue

Gonna be converting this to/from a draft to verify that everything works as intended EDIT: Functionality verified! This not only prevents actions running on drafts, but it will stop a running action if converted to a draft & starts running actions when marked as ready for review!

25-08-07 15-49-22 chrome

@Repiteo Repiteo added this to the 4.x milestone Aug 7, 2025
@Repiteo Repiteo requested a review from a team as a code owner August 7, 2025 20:34
@Repiteo Repiteo force-pushed the ci/draft-skip-actions branch from 7d0183d to 7805120 Compare August 7, 2025 20:36
@Repiteo Repiteo marked this pull request as draft August 7, 2025 20:36
@Repiteo Repiteo force-pushed the ci/draft-skip-actions branch from 7805120 to 9b7a5a4 Compare August 7, 2025 20:37
@Repiteo Repiteo marked this pull request as ready for review August 7, 2025 20:38
@Repiteo Repiteo marked this pull request as draft August 7, 2025 20:39
@Repiteo Repiteo marked this pull request as ready for review August 7, 2025 20:41
@clayjohn
Copy link
Member

clayjohn commented Aug 7, 2025

A little bit of feature creep, but will drafting and undrafting force the actions to run again?

@AThousandShips
Copy link
Member

AThousandShips commented Aug 7, 2025

This feels like pretty unexpected behavior to me, I would expect draft PRs to work just fine, draft status should IMO be about reviews not CI, to ensure things work and are ready for review

There's no substitute for being properly deliberate and intentional about your pushes, if anything it'd be nice to be able to handle too many rapid pushes in a row and delay running if a push happens within a certain window of a previous one, delaying for a bit after that to not start five CI runs that just run a few seconds into building all the platforms

I want my draft PRs to run CI and this would force me to make my PRs non-drafts to accomplish that properly (since not all CI features work correctly on forks at least the last time I checked), which would defeat the main goal namely not triggering requests for reviews

@Repiteo
Copy link
Contributor Author

Repiteo commented Aug 7, 2025

A little bit of feature creep, but will drafting and undrafting force the actions to run again?

Indeed it would! Similarly, an in-progress action would stop when becoming a draft

There's no substitute for being properly deliberate and intentional about your pushes, if anything it'd be nice to be able to handle too many rapid pushes in a row and delay running if a push happens within a certain window of a previous one, delaying for a bit after that to not start five CI runs that just run a few seconds into building all the platforms

Hmm, that'd definitely be a lot trickier. We can't exactly control user intent, and I don't know if there's metadata for determining a previous run.

I want my draft PRs to run CI and this would force me to make my PRs non-drafts to accomplish that properly (since not all CI features work correctly on forks at least the last time I checked), which would defeat the main goal namely not triggering requests for reviews

If there's CI inconsistencies with forks, that should absolutely be addressed. Do you have any specific examples in mind?

@AThousandShips
Copy link
Member

At least a few months ago the static checks don't work correctly on forks, failing to find formatting and spelling errors, this has been the case for years as far as I recall

I've only seen a handful of people who don't expect CI to run on drafts, and the vast majority of inexperienced users don't use drafts, so this won't prevent running in the case of experienced users pushing ten times in an hour

I'd say if someone don't want CI to run they should use the proper tools for it instead, and we should document that better, rather than breaking expectations and hurting usability

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants