🇬🇧United Kingdom @jonathan1055

jonathan1055 → created an issue.

jonathan1055 → created an issue.

jonathan1055 → made their first commit to this issue’s fork.

I've been thinking a bit more about the new variable names, and have the following ideas:

1. Given that the variables are going to be renamed, now would be a very good time to add the leading underscrore, so that all variables which are user-facing and intended to be changed start with _ This makes it simpler to remember, when copying a variable or duplicating a line in .gitlab-ci.yml to add a new onew. The work we did in #3426292: Deprecate and rename some variables that were wrongly named → means that of the non-hidden variables it is only the SKIP_ and OPT_IN variables, plus the one recent new addition RUN_JOB_UPGRADE_STATUS which do not start with underscore.
2. I am not sure of the benefit of having "JOB" and "VARIANT" in the variable name. I know the two types of variable fall into these categories, but it just adds extra length to everything you need to type. I don't think it adds much - people are not going to confuse 'CSPELL' with 'NEXT_MAJOR' as that text gives enough explanation as to what the variable is used for.
3. Following on from (2) I suggest that the "job" variables start with _RUN and the "variant" variables start with _TEST because you "run" a job but "test" against a version/variant. There is no specific benefit in having the varibales all start with 'RUN' when half of them are for specifying a version.

Taking all these three together, in summary it means that the SKIP_xxx variables are renamed _RUN_xxx and the OPT_IN_TEST_xxx variables become simply _TEST_xxx. The _RUN_ variables need to have the value swapped from 1 to 0, 0 to 1 compared to their original SKIP_ value, but the _TEST_ variables keep the same value as their OPT_IN_TEST_ original. This seems easier to explain in the support we'll need to give, and easier for maintainers to convert from the old names.

Two more things to do:

1. Somewhere on the 'home' page, or in the header, it would be nice to have a link back to https://www.drupal.org/project/gitlab_templates →
2. We have duplication in https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → with information that is going to change and we'll have an overhead in maintaining both versions. Do we need to look at what we want to keep in both places and what should be removed from the drupal.org page?

Yes, that's good. As you know, #3449621: Add rule for project-update-bot to test against next major → was committed, but also the default-ref tag was moved to include this, so all contrib that are using the recommended gitlab_templates repo now have the automatic Next Major testing on the project-update-bot-only branch.
https://git.drupalcode.org/project/gitlab_templates/-/tags

The CSPELL job now passes and ends green.
Spell: Files checked: 76, Issues found: 0 in 0 files
https://git.drupalcode.org/issue/geofield-3451574/-/jobs/1742027

MR32 is ready for review

Corrected some more words, and added a few into the project dictionary. Not many left now:

CSpell: Files checked: 77, Issues found: 20 in 9 files
The number of unrecognised/misspelled words is 9

With regard to your comment in #7 that is exactly what the phpstan job shows
https://git.drupalcode.org/project/scheduler/-/jobs/1740052

For #9 that key is is the entity type used in testing, you need the Commerce module.

I added a .cspell-project-words.txt file to contain the specific partially-invented words that are not know by cspell. This further reduced the word count from 53 down to 29

CSpell: Files checked: 77, Issues found: 120 in 31 files
The number of unrecognised/misspelled words is 29

Big improvement on first run, using
_CSPELL_IGNORE_PATHS: 'doc/drupal_org_documentation/Geofield_Mapping_in_Drupal.key'
https://git.drupalcode.org/issue/geofield-3451574/-/jobs/1739903

CSpell: Files checked: 77, Issues found: 262 in 36 files
The number of unrecognised/misspelled words is 53

4369 words reduced to 53

As a baseline, the current CSPELL job shows

CSpell: Files checked: 78, Issues found: 4688 in 37 files
The number of unrecognised/misspelled words is 4369

The log is 8,800 lines long.

jonathan1055 → created an issue.

Before the above core run-tests.sh enhancement is committed we need to get some baseline runtimes. The test runs below are for all tests (except the deleted SchedulerDrushTest) in one job.

31 May 1737240 8 mins 12 secs

jonathan1055 → created an issue.

jonathan1055 → created an issue.

Tested on Scheduler, but from the context you give in the summary, this may not show any useful info. The scheduler phpunit tests are run via a matrix based on the @group in the tests, and javascript tests are in a separate group, as are the kernel tests. So they run in different jobs in the pipeline. It sounds like the chnage would be seen more when the different types of tests are all in the same job

Anyway, testing just the main phpunit group, which has the most tests in it:
https://git.drupalcode.org/project/scheduler/-/jobs/1726041 - with --concurrent 8 duration 8 mins 32

Before, with original --concurrent 32 sample of last 3 jobs, duration 7mins 25, 5mins 58, 6mins 54.

I can modify the phpinit matrix, or remove it entirely, if that would help to get more useful test results.

I have added a new variable _CONCURRENCY_THREADS - is that an OK name?

The default is 8 for now, but we can experiment with doing a calculation.

I removed the quotes around the value, not sure if that is going to cause a problem. Will test and see.

jonathan1055 → made their first commit to this issue’s fork.

Retested after last commit (different backup name) and the job ran as before. Given the comments in #15 by OP @kristiaanvandeneynde and the input from @simonbaese I am marking this RTBC.

Tested with a bad composer.json (added trailing comma) - fails as expected
https://git.drupalcode.org/project/scheduler/-/pipelines/184999

New messages and pwd look good
https://git.drupalcode.org/project/scheduler/-/jobs/1716603

Testing when project has no composer.json file - also runs OK
https://git.drupalcode.org/project/scheduler/-/jobs/1716727

There is no "composer-lint" job in D7

of course! sorry, dumb question :-)

I will check some projects that I'm involved with, but I think they all have a composer.json file. Could fake it, by deleting the file in a prior commit. I will try that.

Thanks for those changes. I have made a few minor suggestions in the MR.
I will also test with Scheduler, and check it works with a project that does not have a composer.json. Is this change going to be ported to main-d7? I think it should be, even though composer.json is not mandatory in D7.

We now have one single command that changes directory into $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME, runs validate and switches back to to the top level $CI_PROJECT_DIR. Because we are validating two composer.json files which can give different results, I think we need to be clearer in the log regarding what is being done. It needs an echo or something to show the second directory that is being checked, or some print to state 'this is your projects composer.json' and 'this is the generated composer.json'. If it's not clear we will get some confused maintainers asking for support. Maybe break the command into a couple of separate statements?

Also, is the --no-check-lock option working? The log still shows Lock file warnings:

$ test -f $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME/composer.json && cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME && composer validate --no-check-lock && cd $CI_PROJECT_DIR
./composer.json is valid but your composer.lock has some warnings
# Lock file warnings
- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`

Interesting. This implies that the parsing and loading of the project's composer.json here in scripts/expand_composer_json.php works just fine and no error was detected.

The un-modified composer.json is restored into $_WEB_ROOT/modules/custom/$CI_PROJECT_NAME so either we run composer validate in that folder, not the top level /builds/project/flexible_permissions, OR we copy the un-modified composer.json back to the top-level at the start of the Composer Lint job (we did this for CSpell)

I have started this, just for one variable, converting SKIP_COMPOSER_LINT into RUN_JOB_COMPOSER_LINT

The first commit just makes the basic variable name change, modifies the .pre stage deprecation job and does the transformation. It is tested here
https://git.drupalcode.org/project/scheduler/-/pipelines/183501

Things to note

1. The deprecation warning shows up for every Composer variant, which needs to be fixed as that is just confusing. It should not be too difficult to solve.
2. The value of the new variable is written to >> build.env so is availbale in all subsequent jobs
3. However, it seems that the rule for determining if the Composer Lint job is added to the pipeline has already been 'executed' by that point, so it is too late to reply solely on the new variable in the rule

The second commit therefore adds back the old variable into the rule. This has the desired effect, it means that the existing $SKIP_COMPOSER_LINT value is respected and the job is not run in this case. Tested here:
https://git.drupalcode.org/project/scheduler/-/pipelines/183952

There may be a better way to do the transformations, so that we can properly remove the old variables now, instead of leaving them in the rule, deprecated, and having to remove them in gitlab_templates version 2.0

Fixed in 2.x. Now for 8.x-1.x

Sorry, yes I meant OPT_IN_TEST_NEXT_MAJOR. In #8 it was a typo, then I copied it to #12 :-)
I would like to test on 📌 Automated Drupal 11 compatibility fixes for jsonapi_extras Needs review as that is the issue which caused bbrala to ask for this feature.

Hi bbrala,
Are you OK if I use this MR to test the changes in #3449621: Add rule for project-update-bot to test against next major → ?
I will revert the test afterwards. Given that you opened that issue, this seems a good place to test the MR.

OK. The phpunit tests for Next Major are allowed to fail anyway, it would just show that the gitlab_templates change is working. It would not affect the pipelines in other issues or the scheduled pipelines, it would just test at D11 for these specific automatic MRs, which is ideal in your scenario. But if you'd prefer, I will find another project to test with.

Would the maintainers of this module be OK if I used this automated MR to test a new feature request of Gitlab Templates? The feature will automatically test the project-update-bot-only branch against the Next Major core version (which is Drupal 11). Currently your tests at Drupal 10 pass, but the point of this automated issue is to test against D11. You could manually opt in to testing against D11 but the feature being developed in #3449621: Add rule for project-update-bot to test against next major → will do this automatically for the merge request generated by the project update bot.

Yes the Scheduler next major can't currently resolve the dependencies (even when I use _LENIENT_ALLOW_LIST and include every dependency the module has. But the fact that the job ran even when OPT_IN_TEST_NEXT_MINOR=0 shows that the branch name check seems to work. Is there another project we can test it on, or is this enough?

I've changed the rule to use CI_COMMIT_REF_NAME

Testing here - with this MR and OPT_IN_TEST_NEXT_MAJOR set to 0 to prove that the updated rule turns testing on for this
https://git.drupalcode.org/project/scheduler/-/pipelines/183428

Hi @Rajeshreeputra,
I have removed your manual commits, as I want this issue to remain only for the automated Project Update Bot suggestions. If you would like to help please create separate issues to fix the different problems.
Thanks

jonathan1055 → made their first commit to this issue’s fork.

Looking at the environment variables CI_COMMIT_BRANCH is not shown, so is blank. But there are two other possilble variables

CI_COMMIT_REF_NAME=project-update-bot-only
CI_COMMIT_REF_SLUG=project-update-bot-only

MR43 now has OPT_IN_TEST_NEXT_MAJOR: 1 so using that MR to test this change is not going to demonstrate the enhancement.
https://git.drupalcode.org/project/jsonapi_extras/-/merge_requests/43/co...
I suppose @Rajeshreeputra did not know you were using that MR for testing this one. Let me know if you do not want to revert those changes, and then we will find another project to test this against.

Thanks for starting work on this. The change seem to remove the PHPStan warning, compare the updated job with scheduled pipeline job

However, the nightwatch tests now fail, whereas on the scheduled pipepline they pass.

The changes look good but I've not tested it. Nice to have the list is a variable.

Can I ask why the second adjust variable is callled DRUPAL_PHP_FILE_TYPES_SLASH as that impies the values would be "php/module/install/ etc". Can we call it something like DRUPAL_PHP_FILE_TYPES_BAR or DRUPAL_PHP_FILE_TYPES_PIPE or something other that SLASH ?

Just a quick question. The final change to the docs added
"If you are testing with Drupal 10 (which uses PHPUnit 9), your project needs to have a PHPUnit configuration file to be able to get deprecation warnings."
I think this is only true if you are running phpunit direct via _phpunit_concurent=0. Scheduler does not have a phpunit.xml but I can get deprecation warnings just by setting SYMFONY_DEPRECATIONS_HELPER: "ignoreFile=$CI_PROJECT_DIR/$_WEB_ROOT/core/.deprecation-ignore.txt" provided I also use _PHPUNIT_CONCURRENT: 1

So somehow, in the docs page, this needs to be expained.

Thinking about this again, it will be too late to do the variable transformations in .create-environment-variables because that is executed in the Composer jobs, and so the new variables used in the composer rules: will already need to be set to the correct final values. So I think it will have to be done before any Composer job, which probably means in the .pre stage?

As for variable precedence, I guess that if we find a newer variable, that should take priority I think. The changes about the "0" to "" should make it easier.

In principle it would be nice to have the new variable value take precedence, but I don't think this will be possible in the scenario we are working in. I don't know any way that we can tell if a new variable has been set in the project's .gitlab-ci.yml file or is just taking the default value. But we can be sure when an old variable has been set in the project's file, because it will have a non-blank value. So we use that value to set the new variable. The other new variables are not touched, they will either be set in the project's file or will take the default.

I like this approach. I can see why the OPT_IN_ and SKIP_ names got set that way during the initial development, but having the name contain a meaning which implies a default value does cause us problems when we want to start new things, as you have said with the upgrade_status job.

Setting the new variable values from the existing could be done in .pre but it could also be done in the .create-environment-variables section. We can see which is more appropriate when we start the MR. The decision on whether to give a deprecation warning or not may also influence the place where the new values are created.

When setting the new values, we could change the default values of the old variables to blank and move them into the hidden variables file. Or maybe just delete them completely. Either way, if we find a value (either 0 or 1) we know that it was set in the project's .gitlab-ci.yml and can take the appropriate action. What happens if someone has both the old and new and they conflict? for example OPT_IN_NEXT_MINOR: 0 and RUN_VARIANT_NEXT_MINOR: 1. I guess we decide which takes precedence. But given that we can't tell immediatly if a new variable was set in .gitlab-ci.yml or just defaulting, we might need to check the value against the default, in order to determine which to do. Or maybe we go with the old value, as if that has a value we know it has been set explicitly in the project file, and we shold respect that.

I made a couple of MR suggestions to the help text.

What is your plan for releasing a full 2.0?
Currently 2.0.0-beta supports ^8.7.7 || ^9 || ^10
If 3.0 is going to be for D10 and D11 only, then can the 2.x series remain compatible with 8/9/10 as currently?

If the search_api_solr tests are intrinsically linked to the search API tests, you can trigger a downstream pipeline in gitlab, so that when you test search_api you also test solr. Testing on every MR run would be over the top, but you can set the downstream test suite to be triggered manually. So when everything was ready, just before merge, you would also start the Solr tests, for a final check. That would help to avoid this kind of fault.

This is ready for review again. I forced an error by supplying a bad value for _UPGRADE_STATUS_ANALYZE_EXTRA. The job fails red now, exectly what we wanted.

https://git.drupalcode.org/project/scheduler/-/pipelines/177276

I added echos to show what is being executed, and to show the exit_code value. Both of these match what we have for other jobs.

Ah, I could not see that code suggestion. Sounds like that is perfect - it will end with amber warning if code is 3, but red 'fail' for other non-zero codes. I've made that change and the job now ends amber when it has run OK but found issues
https://git.drupalcode.org/project/scheduler/-/pipelines/177207

I will also try to simulate a bad fail, any ideas? It might need a temporary customisation the job script.

Tested again
https://git.drupalcode.org/project/scheduler/-/jobs/1635741
I read in the email notifications (but could not find it on the site here in MR comments) that there are several return code values >0. This seemed to be the reason why you deleted allow_failure:true. I dont see why we have to delete that, because ending amber would give the same kind of information, and maintainers are not necessarily expecting their projects to pass. Ending red makes it look like something bad has happened. I know that we can add allow_failure:true into the project's .gitlab-ci.yml but I still think that this should be the default for this job. At least at the moment it should be. At some point in future, say when D11 real release is imminent, we could change it false.

FYI unless there's a reason to keep I removed on the 3.0.x branch

Sorry I don't get what you mean. Removed what on the 3.x branch?

The fix added

$entity = $event->getNode();

but immediately after the new conditional check, we already had

$entity = $event->getEntity();

Is there a reason why we have to use getNode()? It seems wrong (a) to have both calls, and (b) it should be content-type generic, not just for nodes.

Putting this back to 'active' to that it does not get 'closed (fixed)' after two weeks.

Given that I've done the changes in the MR we might as well commit this.
When did we drop 8.x-1.x from the supported list? I could be on there, as it does all run OK.

Thanks for breaking the Solr tests again ;-)

?

The 'rebase' button is no longer shown, but it can be done with a special /rebase comment

I've pushed some changed

Default, with only opt_in_next_minor: 1 the job does not run (and neither does 'composer')
https://git.drupalcode.org/project/scheduler/-/pipelines/170974

Just adding _run_upgrade_status: 1 we now get the current 'composer' job which is the prerequisite for 'upgrade status'
https://git.drupalcode.org/project/scheduler/-/pipelines/170974

The docs will need updatiung once we settle on the approach, as I have removed the references to SKIP_UPGRADE_STATUS.

This a good point. Maybe it is missing in the documentation on
https://project.pages.drupalcode.org/gitlab_templates

We have an open issue for reviewing the doc pages, so if you think this would be useful to add, or make more clear, please do make a suggestion on #3439644: Review all documentation pages →

Fixed on 2.x
But NW for 8.x-1.x

I made a couple of suggestions in the review. I was going to just push the change and test, but nicer to suggest first.

I've created a new branch for the next set of doc pages review/updates, therefore setting issue to active, rather than let it get closed. I might not have time to do much for a few weeks, so wanted to keep this issue open.

jonathan1055 → changed the visibility of the branch main to hidden.

@alexander I don't think we can delete issue forrk. On slack

https://drupal.slack.com/archives/CGKLP028K/p1705709051064799

Try the #gitlab channel, that's a better place for discussion I think, rather than this documentation page

quietone → credited jonathan1055 → .

The current MR changes 123 files, I've not re-read it again in detail, but it seems good to get those fixed now and not wait on the two subsequernt Coder sniff issues. If we wait, there will be a lot of conflicts/manual fixing again, and probably more new incorrectly formatted ones. Let's not waste the work done here so far, but get it looked in and committed. Doing that now will not make it any harder to fix the finer points when those later issues land.

Due to the differing eslint config rules, if the files are fixed for eslint on GitLab pipelines, they fail on DrupalCI. So the .eslintignore file will stay in place (to make sure the DrupalCI eslint does not fail) but the file is deleted in a custom composer: after_script: so that the three ,js files are ignored in the gitlab eslint job.

We now have test coverage for the above problem, added in
📌 Expand javascript test coverage Fixed

We now have test coverage for the javascript behaviors summary updates when editing an entity and when editing an entity type.

Thanks. Your change is good. Here's a collage of screenshots showing the new reduced form. It was only 3 1/2 screens, but previously is was 10 1/2 screens. A few variables are not showing in my screenshot because I have them in the .gitlab-ci.yml file. So it is probably 4 screens compared to 10.5. That's still a nice % reduction.

My comment in #6 is timestamped 3 minutes after yours in #7 oddly, so maybe you did not see it. Yes, that's the intention, and all seems fine.

Yes, moving DRUPAL_CORE out of .composer-base and into the top-level variables is a good improvement, and means that the value is set for all jobs. But for the variant phpunit / phpstan / nightwatch, the value is incorrect because it remains set to this default.

I tested MR201 and you can see in https://git.drupalcode.org/project/scheduler/-/jobs/1506862 that all of the phpunit jobs have drupal_core 10.2.x-dev which is misleading.

I thought that my idea in #2 was not going to work, but I was mistakenly thinking that the build.env file was the same for all jobs. Of course it is not! It is distinct per build job, and is used in each job that subsequently 'needs' that specifc composer build. This is perfect, it's exactly what is needed, and was no doubt designed exactly this way.

So in create-environment-variables we write the value of DRUPLAL_CORE to build.env, just like the other variables, then it is automatically correct for variant versions of any job now existing, or any new ones we create.

The same can be done for PHP_VERSION and then we can delete the specific variables: rows for all the variants. This is more robust, because we are only setting the value once, rather than also having to set it in all the phpstan / nightwatch / phpunit variants.

In these phpunit jobs https://git.drupalcode.org/project/scheduler/-/jobs/1519702 each one now shows the correct DRUPAL_CORE

Ready for review.

And thank you for asking the question in the first place. The enhancement in #3442293: Make it easier for contrib modules to specify additional CSpell flag words → was good to do.

jonathan1055 → changed the visibility of the branch 3441816-check-version-of-core to hidden.

The trailing comma is incompatible with PHPUnit 9.6.19 in Drupal 9.5 PHP 7.4. Therefore removed the comma and added

// phpcs:ignore Drupal.Functions.MultiLineFunctionDeclaration.MissingTrailingComma

MR190 can be closed, now that all work is moved to MR197 on #3343522: Hide variables that should not be overridden →

MR197 is ready for initial review and feedback. Comparing the old and new files directly is not straight-forward due to the value: moving, but I have taken local copies of both, and wrangled them to make a compare work, and the values have all been set the same as before.

Pipeline test
https://git.drupalcode.org/project/scheduler/-/pipelines/164204

I had previously done some investigation on which of the CORE_ version variables are actually used anywhere in this project (apart from in check-versions.php). So while we are discussing the variables, I thought it was sort-of relevant to include this here, as it may inflect the decisions.

Variables used

Variables not used

(values accurate as at 4 May 2024)

This job is run in the scheduled pipelines every day. Do we need that? It would only show anything useful if a coding standards or quality check rule was changed. But we would soon discover that scenario on the next MR. I think we could save resources and only run it for merge requests or commits, by adding

  rules:
    - if: $CI_PIPELINE_SOURCE == 'merge_request_event' || $CI_PIPELINE_SOURCE == 'push'

Removing the spelling error, number of failed checks: 3
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1508358

Remove the phpcs error, number of failed checks: 2
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1508434

Fix shellcheck - number of failed checks: 1
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1508563

Fix eslint / yml - number of checks that failed: 0
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1508731

Ready for review.

The commands for each of the four jobs are now grouped in a | so that the output is easier to read. Also incremented the exit_code to give total number at the end. This test shows that all 4 checks failed.
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1508105

I have a new issue 💬 [ignore] Testing gitlab templates MRs Active on Scheduler which is being used for Upgrade Status testing with MR199 (so you can ignore the link to issue 3266795 in comment #21 above)
The pipeline is https://git.drupalcode.org/project/scheduler/-/pipelines/163386

jonathan1055 → created an issue.

This job now reports errors in all four of the jobs, which is what I wanted to see.
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1499133

totally missed #7

that was my fault in putting too many reviews/fixes together.

Tested with SKIP_UPGRADE_STATUS: 0 and it all works OK.
https://git.drupalcode.org/project/scheduler/-/pipelines/163243

Yes see my comment in #7. The OPT_IN_ series of variables are all related to Drupal core version and PHP version variants, and it would be confusing to have this new job controlled by a variable in that set. The SKIP_ control whether a particular job is selected, and even though these all so far have had a default of 0, there is no problem in having a default of 1 for SKIP_UPGRADE_STATUS. At some later date, it could even be turned on for all projects by changing the defaut.

To demonstrate the problem the first commit added a blank line that eslint/prettier would report on
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1497401

The second commit added a spelling error, and this is detected, but the eslint problem is no longer shown
https://git.drupalcode.org/issue/gitlab_templates-3444921/-/jobs/1497546

Ah, I thought you had removed the anchor because I read the extra the commit in #30 first, then the commit in #31, and just assumed that they were done in that sequence, i.e. comment #31 was after comment #30. But actually the commit in #31 was before the commit in #30.

jonathan1055 → created an issue.

If we have to add one (or two) new OPT_IN variables we need to avoid having both 'PREV' and 'NEXT' in the variable name :-) It's confusing enough, and that would just tip people over the edge. PREV_MAJOR_LTS is a big improvement, and it seems we only need to go back a version, we don't need PREV_MAJOR_NEXT_MINOR

Yes, that worked and we now get the new job
https://git.drupalcode.org/project/scheduler/-/pipelines/162662

I changed the value: keyword to lower case and that fixed the syntax. The pipeline is created
https://git.drupalcode.org/project/scheduler/-/pipelines/162644

However there is no 'upgrade status' job, even though I set OPT_IN_UPGRADE_STATUS: 1.
I think the rules

  rules:
    - *opt-in-current-rule
    - *opt-in-upgrade-status-rule

might also need

    - when: on_success

Also, you have called the new variable OPT_IN_UPGRADE_STATUS: 0 but we have generally used the OPT_IN set of variables for core version/php version variants. Would it be better to use SKIP_UPGRADE_STATUS: 1 instead?

Just tried a pipeline and the yml syntax needs looking at
variables:_upgrade_status_analyze_extra config uses invalid data keys: value
https://git.drupalcode.org/project/scheduler/-/pipelines/162541

I see you removed the anchor in jobs.md#skip-and-opt_in-variables. Did it not work correctly? It is hard to check the links when we don't actually publish the pages within the MR (and I don't have it mkdocs running locally).

From #62, in relation to the two things not covered by the sniff, there are already existing Coder issues for both of these:
📌 Create sniff for checking the indentation of lines that follow an @todo comment Active
📌 @todo should start with capitalized letter or non-alpha symbol Active

Just reporting here for info, and to help others, that the change to add a trailing comma is not compatible with PHPUnit 9.6.19 in Drupal 9.5 PHP 7.4 (which is what you get when using OPT_IN_TEST_PREVIOUS_MAJOR on gitlab pipelines). The phpunit test complains of a syntax error and gives

ParseError: syntax error, unexpected ')', expecting variable (T_VARIABLE)

So if you want to fix the PHPCS rule and still maintain the ability to test at 'Previous Major' you can ignore the rule on the next line:

    ModuleHandlerInterface $module_handler,
    // Trailing comma is incompatible with PHPUnit 9.6.19 in Drupal 9.5 PHP 7.4.
    // phpcs:ignore Drupal.Functions.MultiLineFunctionDeclaration.MissingTrailingComma
    EntityTypeManagerInterface $entity_type_manager
  ) {

Sorry, just to be clear, I'm not trying to take the work over. It's assigned to you :-)

The suggested script in the issue summary does work. Could also add in the current dictionary, then could diff to make a patch for it. I used:

export WORDS=dic-words.txt
export DICTIONARY_FILE_PATH=dic.txt
export DICTIONARY_FILE_PATH2=dic2.txt
echo "Aa\naa\nAA" > $DICTIONARY_FILE_PATH 
cat $DICTIONARY_FILE_PATH $WORDS
cat ${DICTIONARY_FILE_PATH} $WORDS | tr '[:upper:]' '[:lower:]' | LC_ALL=C sort -u -o ${DICTIONARY_FILE_PATH2}

There are some other choices for LC_ALL, so we need decide on that.

We have another proposed way to hide the 'do not modify' variables and this issue is the best place to continue the work I started on #3441816-57: All variants run the same Drupal version when the pipeline is triggered via web →

Like fjgarlin, I was also confused by the request. I'm not saying that there is no problem, I just need some help to understand what it is. When we get 11.0, using the OPT_IN variable names to assist my thinking, we will provide contrib testing for the following:

OPT_IN_TEST_CURRENT = 11.0
OPT_IN_TEST_NEXT_MINOR = 11.1.x
OPT_IN_TEST_NEXT_MAJOR = 12.x (when it exists)
OPT_IN_TEST_PREVIOUS_MINOR = 11.0 (when CURRENT has moved on to 11.1)
OPT_IN_TEST_PREVIOUS_MAJOR = 10.3

Are you saying that we need to provide testing at more D10 versions, other than the one specified in previous_major?

You're welcome. It was a good collaboration.