[CI/CD] allow execution of CI/CD steps for PRs conditioned to PR title

[picnixz]

Related: #12115.

[danieleades]

Related: #12115.

@danieleades Could you confirm that I won't be destroying master?

ha that's a lot of pressure

Sorry, i know this is a bit of a step backwards, but are we sure that the additional complexity this introduces is worth the benefit?
in the balance of tradeoffs, unless the time saved is significant, usually simpler is better

[picnixz]

Sorry, i know this is a bit of a step backwards, but are we sure that the additional complexity this introduces is worth the benefit?

Sometimes, you really want to just check that your branch is fine on Windows (because you have weird stuff). But in general, all Ubuntu checks are done beforehand... but you know that they will be fine... so you don't want to wait them to complete. Technically, this workflow would simply be triggered to synchronize the windows-testing branch but otherwise I can keep doing it manually.

EDIT: In general, we gain 2 minutes on the tests.

[jayaddison]

@picnixz I wonder whether we could do this by matching on the source-name branch instead?

(that would indirectly also ensure that people (such as me, for example) trying to fix/identify Windows-specific bugs using CI would be less likely to accidentally write into their normal development branches)

[jayaddison]

@picnixz I wonder whether we could do this by matching on the source-name branch instead?

(that would indirectly also ensure that people (such as me, for example) trying to fix/identify Windows-specific bugs using CI would be less likely to accidentally write into their normal development branches)

I think that the on.[push|pull_request]: ... condition may be limited to positive-filtering (branch name like x) but here we want to disable other tests when a branch name does not match.

However, we may be able to achieve the same effect by using an if: ... expression on each relevant job -- similar to an existing pattern we use here:

sphinx/.github/workflows/main.yml

Line 146 in 982679e

if: github.event_name == 'push' && github.repository_owner == 'sphinx-doc'

For fairness I'd suggest adding both if linux and if windows expressions -- assuming this approach is possible.

[jayaddison]

For fairness I'd suggest adding both if linux and if windows expressions -- assuming this approach is possible.

Perhaps those should be (pseudocode again): if branch_os is empty || branch_os.startswith(linux/) and if branch_os is empty || branch_os.startswith(windows/), respectively.

[jayaddison]

I'm not sure that my suggestion is feasible; I think that when a commit is pushed to a pull request branch, it is the pull_request event that is initiated.

The GITHUB_REF variable for the pull_request event contains a git ref of the format:

refs/pull/<pr_number>/merge

(push events do include branch name -- but they're for cases where a process/user with write access writes directly to the repository -- not an external contributor pushing to a branch in their own repo)

[jayaddison]

Although my suggested alternative doesn't seem valid, I think that maintaining and additional branch for the purposes of continuous integration filtering could produce unintended confusion and complexity.

I'll continue to try to think of and feasibility-check an alternative. Could labels be used to selectively disable workflows on-next-push?

[picnixz]

I think that the on.[push|pull_request]: ... condition may be limited to positive-filtering (branch name like x) but here we want to disable other tests when a branch name does not match.

This can be done with an if inside the job. But then it would mark "job skipped". Do you think it's fine like this? (I'm not sure if the job setup time is negligible or not).

I think that when a commit is pushed to a pull request branch, it is the pull_request event that is initiated.

I checked and every commit triggers a pull_request event. So I could have a job that conditionally run the whole matrix if the pull request branch has a specific title.

I'll continue to try to think of and feasibility-check an alternative. Could labels be used to selectively disable workflows on-next-push?

Not a good idea since not everyone can put labels.

[jayaddison]

I think that the on.[push|pull_request]: ... condition may be limited to positive-filtering (branch name like x) but here we want to disable other tests when a branch name does not match.

This can be done with an if inside the job. But then it would mark "job skipped". Do you think it's fine like this? (I'm not sure if the job setup time is negligible or not).

I think it's OK, and it might even be preferable: it's a hint to the reviewer that not all tests were run for the latest commit. That could be relevant before deciding to merge or not, for example.

I think that when a commit is pushed to a pull request branch, it is the pull_request event that is initiated.

I checked and every commit triggers a pull_request event. So I could have a job that conditionally run the whole matrix if the pull request branch has a specific title.

I'll continue to try to think of and feasibility-check an alternative. Could labels be used to selectively disable workflows on-next-push?

Not a good idea since not everyone can put labels.

It's a fairly rare use-case at the moment though, so I think that requesting triager/maintainer involvement could make sense, before using our CI to run a lot of test compute.

[picnixz]

I think it's OK, and it might even be preferable: it's a hint to the reviewer that not all tests were run for the latest commit. That could be relevant before deciding to merge or not, for example.

Then I'll take that approach instead. Not sure if it'll be working as expected but we could always revert the commit (I mean, this one won't break master for sure !). I'll work on it today.

[picnixz]

I think I will try to create a real branch on Sphinx and do some PRs to check what happens. Then I will merge this branch into the real master (but first, time to eat).

[picnixz]

What do you think of this approach @danieleades ?

[picnixz]

Possibly related (and blocking): actions/typescript-action#27

[picnixz]

I also would like to know how I can have a required step that checks that the PR title does not contain any '[ci-skip-*]' strings.