Fix CI on main branch

[sbidoul]

fixes #9782

[sbidoul]

For pull requests it's not necessary to checkout the code

But in other circumstances than pull requests it seems it is, so let's checkout always for the sake of simplicity

[pradyunsg]

This won't run all the jobs when the most recent commit only changes a subset of the files (say, just a news file or just the docs).

I have a fix locally. Lemme push that into a separate PR.

[sbidoul]

Are you sure the complexity of this determine-change is worth it ?

[pradyunsg]

Oh, I never updated #9814. 😅

[pradyunsg]

Are you sure the complexity of this determine-change is worth it ?

Yup yup! Saving ~30 minutes on documentation-only PRs, or PRs that don't change code is worth it IMO.

[pfmoore]

Are you sure the complexity of this determine-change is worth it ?

I have the same question. I feel that our CI is getting too hard for mere mortals like myself to maintain, which increases the risk of problems remaining unfixed because the people who know how to fix them aren't available.

[pradyunsg]

our CI is getting too hard for mere mortals like myself to maintain

Ah interesting!

I felt like we were heading the other way, since our CI is now "simplified" down to a single file (+ the setup-ram-disk stuff) on a single provider. That's very different from what we had earlier, spread across 3 providers and 10+ files.

The only complexity from my PoV is that we have some additional logic to not burn CPU cycles on running tests if we have a non-code PR; which I personally think is worthwhile -- since it's at the cost of a small job and a few "if"s in a couple of jobs?

[pfmoore]

I agree that one provider is an improvement. And we had some funky logic in the old CI, too, IIRC, so maybe it's a case of "what you know is no longer scary". And having looked at the CI spec in detail, it's probably true that it's mostly lack of familiarity.

I suppose things I don't understand are as follows:

• The whole business of "For pull requests it's not necessary to checkout the code". I've never really understood what the starting environment is for a github action, so this makes me feel that we're doing something "advanced" because I've always got along fine without needing this knowledge before.
• I'm not a fan of having to add "if it's not a pull request" all over the place. If we're saying "need to run the tests" can't we include the logic "we always need to run the tests if it's not a PR" in that flag? None of the examples in the docs for that GH action suggest that this is needed.
• Also, wow, that New-RAMDisk.ps1 script. It's entirely a good thing, but can we at least include a comment that links to where that came from? I assume you didn't write it from scratch, and I'm sure I don't know enough to fix it if it goes wrong 🙂

Maybe the real takeaway here is that our CI is sufficiently complicated that it needs some documentation in its own right?

Sorry for not picking up on this when the changes to the CI were added. I tend not to notice infrastructure-type PRs unless I'm pinged, or there's a particular comment that stands out to me.

Also, and I get that this is just how github actions are, how do we know that this dorny/paths-filter action is a good thing to use, and not just some random person's code? It's the same reservation that I have with Go's "github as the package index" approach, so it's definitely just a general comment here and not something wrong with our CI per se. But do we need any sort of guidance on what GH actions it's OK to use in our CI?

[pradyunsg]

And having looked at the CI spec in detail, it's probably true that it's mostly lack of familiarity.

Yea, I don't think there's any way to fix this TBH, beyond having everyone else get used to it.

Sorry for not picking up on this when the changes to the CI were added.

No worries. :)

Let's capture all of this in an open issue though, instead of a closed PR.

[pfmoore]

Done. #9829