-
Notifications
You must be signed in to change notification settings - Fork 697
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
split "validate old ghcs" into a separate workflow #10274
Conversation
41bd31a
to
a4aace8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! A clear win, to my eye
Does anyone know how to make the RTD job rerun? It's not available in the GHA jobs queue, and it looks like the commit hadn't propagated fully or something when RTD's builder tried to pull it. (ETA: now irrelevant as rebasing on |
Also, strictly speaking, the post job isn't required because there's only one job defined otherwise. Keeping it makes it consistent with the bootstrap and validate jobs, and allows for adding things to the new workflow later (not that I can think of anything we'd want to add). |
a4aace8
to
68595ad
Compare
Would it be possible to un-tag me in the commit message? I'm getting a standalone (not chained) email notification on every update of the commit, which is a little annoying. |
Should be untagged now. |
One possible remaining issue: the cache rule's comment suggests that it is expecting the cache from the regular validate rules, which may mean that it needs a cache to be created first. I'm not sure when or how this would manifest, though. Actually, looking at it more, I think we're expecting |
68595ad
to
96a8917
Compare
You changed it in the PR description but not in the commit message, so it had no effect 🥲 Cache is a black magic to me, sorry. If it's too much of a pain, perhaps, we should reconsider the whole thing... |
456963a
to
abdf69b
Compare
Is it possible to not use cache at all in this job? It runs on Ubuntu only, so it will still, perhaps, be faster than windows and mac jobs on the main validate job, and that's all that we should care for: the new workflow doesn't slow down the overall CI time. |
It's definitely fast enough; that's part of how I realized cache wasn't doing what was intended. What worries me is wasting (what I consider to be) precious CI resources. |
ce92d86
to
1fd16d7
Compare
96fc81c
to
fdc0e14
Compare
I am not convinced this is a good idea, for the same reason that Artem wants it: complexity. I'm very worried that this will break things, although as far as I can tell it should be okay as long as nobody commits anything overriding branch protection. All branch protection rules will need to be updated to check for "Validate old ghcs post job" the same way they check for "Bootstrap post job" and "Validate post job".
fdc0e14
to
cc16e90
Compare
I'm parking this. The waste of CI resources is one thing, but the real problem is we can't check that this workflow completed successfully in But we may want that anyway, depending on what we want to do with #10372. |
Especially considering #10379, we need to keep these in the main validate run and improve our testing thereof. |
I am not convinced this is a good idea, for the same reason that Artem wants it: complexity. I'm very worried that this will break things, although as far as I can tell it should be okay as long as nobody commits anything overriding branch protection.
All branch protection rules will need to be updated to check for "Validate old ghcs post job" the same way they check for "Bootstrap post job" and "Validate post job".
Template B: This PR does not modify behaviour or interface
E.g. the PR only touches documentation or tests, does refactorings, etc.
Include the following checklist in your PR:
NOTE re backporting: branch protection rules need to be updated, but branch protections are currently applied to a rather widely-matching pattern and I don't think we want to backport this to every 3.x branch. It would be possible to restrict this rule to
3.1*
because we have no 3.1 release, but then we need to backport to 3.10 branch in case a security problem is found that forces us to make another 3.10 release or something similar.