Add a task for phpstan previous major

Still to do:
- Document the new job so it appears here: https://project.pages.drupalcode.org/gitlab_templates/jobs/
- Discuss the option to turn this new one on/off in isolation or just use the same derived rules as the other jobs

The MR can be tested in projects already regardless of the remaining tasks: https://project.pages.drupalcode.org/gitlab_templates/info/testing-mrs/

Thank you for adjusting the order as per this comment → . I saw the first commit and was going to ask for that, but you did it anyway.

I think in principle we should keep it simple and the default will be the same as in the existing rules, without a separate variable. The only concern is that it may lead to phpstan failures where things have moved forward (eg. new rules, new phpstan version) which no longer match with Previous Major. Or maybe it will be OK as the PHPStan version will not change for Drupal 10. Happy to go with this initially, and if we get lots of complaints then we can do a follow-up.

Cool. My thinking was the same here. Keep it simple and see if we need to make any adjustments.

I assume that the version of Phpstan brought will match the correct one for the Drupal version being checked.

In that case, the MR would be fully ready. I triggered some downstream pipelines here: https://git.drupalcode.org/project/gitlab_templates/-/pipelines/553793

I had edited the above comment #6 to add the versions before I saw #7

Tested on Scheduler and all the PHPStan jobs passed
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/553870

I will also test the OPT_IN variable, but the code looks correct.

Tested with OPT_IN_TEST_PREVIOUS_MAJOR: 0
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/553898

Tested with SKIP_PHPSTAN: 1
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/553902
We still get the composer jobs, even though every lint and testing job has SKIP: 1. But that's how it is, not any fault of this MR.

RTBC

Thanks for the extra testing. Merged.

Wow, this is a high-speed turn-around. Thank you so much. Very grateful for this improvement.

It was an easy one and probably one that should have been from the beginning, but it's hard to anticipate the needs before they happen.

I wouldn't say that "It should have been in at the beginning ..." as I thought the original design was only to run linting/code quality on current and future versions, and not past versions. I'm pleased we have added it now though.

Yeah, that was probably the correct thinking when D10 was the current and D11 was still under development (current + future). But as we have now D11 in addition to D10 being supported, the "current" part gets a bit blurry, so it makes sense now to do current + previous.

We are all in the same page in any case. Great addition!

Automatically closed - issue fixed for 2 weeks with no activity.

I've now had to fix two projects to remove the PHPStan Previous Major job when the "current" is defined to be Drupal 10 and "previous major" is Drupal 9. The phpstan package does not exist by default in the Drupal 9 images. Maybe we should add an extra criteria in the rules for 'phpstan previous major' and not run the job if the core version is less that D10 ?

@fjgarlin (or any other maintainer) could you re-open this issue please so that it shows again in the list.

My first attempt, adding

    - if: $DRUPAL_CORE =~ /^(8|9)/
      when: never

failed to prevent the job being added. I think this is because $DRUPAL_CORE is a derived variable, as it has to differ across the variants. Therefore it is not defined at the time the pipeline and jobs are created, so the condition is not true.

The variable is usable in script, at runtime, so another option is to exit with 0 if the phpstan binary is not found when running on D9, then at least we do not fail the job and the pipeline remains green.

MR398 is ready for review.

Here's the example in D9
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/62...

The suggested changes look good.

Merged.

Automatically closed - issue fixed for 2 weeks with no activity.