🇬🇧United Kingdom @jonathan1055

That's a good idea. It is actually runTasks() where the error is reported, when trying to create drupal_install_test so we need to get info right before that happens.

Put back 'current' and we get a failed job right away
https://git.drupalcode.org/issue/api-3537328/-/jobs/5972397#L536

So it's not parallel: that causes a problem, but it could be two or more variants running at the same time (wild guess)

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.

When running just one variant the job passed first time
https://git.drupalcode.org/issue/api-3537328/-/pipelines/553955
(obviously this is not significant in itself, need to run many more times to verify that it never happens on one variant)

with parallel: removed phpunit previous major failed with the database problem
https://git.drupalcode.org/issue/api-3537328/-/jobs/5971675

simple re-run passed
https://git.drupalcode.org/issue/api-3537328/-/jobs/5971810

jonathan1055 → changed the visibility of the branch 3537328-mr326-database-name to active.

Ignore the typo in the branch name, it should be 3537328-mr362-database-name not 326.

jonathan1055 → changed the visibility of the branch 3537328-mr326-database-name to hidden.

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

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

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.

A new Scheduler pipeline got one failure
https://git.drupalcode.org/issue/scheduler-3445052/-/jobs/5971134#L620
But a simple re-run passed.
https://git.drupalcode.org/issue/scheduler-3445052/-/jobs/5971213#L551

It can't be a proper bug/fault in any code if the problem appears to be random and re-running fixes it. I was wondering if it is caused by a race condition or some conflict due to timing? That made me think if we have ever seen this failure when the phpunit job is run as one single job, i.e. not in parallel as a matrix of test groups?

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.

So do you think it is a permissions problem? The new drupal_database does appear to still exist at the end of the job, so the message Database drupal_database not found. The server reports the following message when attempting to create the database: SQLSTATE[HY000]: General error: 1007 Can't create database 'drupal_database'; database exists. still confuses me.

It seems that this has already been fixed in 📌 Fix cspell and phpstan builds Active on the 2.1.x branch. If I was able to view the pipeline logs I could have discovered that.

In Scheduler, the Address module is brought in as a 3rd-party dependency from Commerce, and this currently only uses Address 2.0.4 released May 2025, but this fix is not included in that. Is there a plan to back-port the fix to 2.0.x ?

jonathan1055 → created an issue.

This correction has also removed the warning No tests found in class "Drupal\Tests\token\Kernel\LanguageTest which means that tests in this class are now being run (they were not, before). We get the extra line

Drupal\Tests\token\Kernel\LanguageTest                          30 passed

I've compared the logs before and after, and all other test results are the same.
So this is not just about fixing the warning, but also it restores test coverage for the LanguageTest.

The warnings in PHPUnit no longer show
https://git.drupalcode.org/issue/token-3537090/-/jobs/5958549
The phpunit errors are for other exitsing problems. The tests still pass green in Previous Minor (11.1) and Previous Major (10.5)

and the PHPStan mesages are also fixed
https://git.drupalcode.org/issue/token-3537090/-/jobs/5958544

This is ready for review.

jonathan1055 → created an issue.

I ran this on Scheduler and I did get one of the errors as seen before
https://git.drupalcode.org/issue/scheduler-3445052/-/jobs/5946963#L610
It was not using this MR but just set

.test-variables:
  variables:
    MYSQL_DATABASE: my_own_db

but I also added the mysql debug output. It all looks fine.

Then re-ran the job and it passed
https://git.drupalcode.org/issue/scheduler-3445052/-/jobs/5946986

Thanks. I've changed to --host=database but got ERROR 1045 (28000): Access denied for user 'root'@'127.0.0.1' (using password: YES). Then I realised the user should be $MYSQL_USER not root. Now it works.

https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

But recently we have not had any failures like we've seen in #23 and #32. That was the reason for adding these debug lines in the first place.

running the command from the mysql binary directly

OK. That's a good idea. First I added
mysql --user=root --password="$MYSQL_ROOT_PASSWORD" --host=mysql --execute "show databases;"
but this produces
ERROR 2005 (HY000): Unknown MySQL server host 'mysql' (-2)

I changed the host to 'localhost'
$ mysql --user=root --password="$MYSQL_ROOT_PASSWORD" --host=localhost --execute "show databases;"
and got a different error
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)

Then without any --host
mysql --user=root --password="$MYSQL_ROOT_PASSWORD" --execute "show databases;"
we get the same
ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/run/mysqld/mysqld.sock' (2)

I've tried to search for examples, to save me guessing, but not found it yet. I'm sure it is simple if you know.

From #15

This may also help to keep the projects values ahead of the defaults, thus removing the need for the two exceptions.

As expected, the change to repeat the project's json array at the start of the merge does in fact solve both of those ordering overrides. So we can delete those two lines.

There may be a downside to using $preserve_integer_keys = TRUE if we ever added new numeric (non-text) array values to the $json_default array. But we could document that, and make sure its avoided.

However, I found a nice simple and neat way to preserve the input order of the array. deep_merge can take multiple arrays, so instead of merge_deep($json_default, $json_project) we just repeat the project array at the start - merge_deep($json_project, $json_default, $json_project); and then use the $preserve_integer_keys = TRUE parameter to avoid duplicating arrays that do not have a named text key. The result is the much cleaner version as attached. This may also help to keep the projects values ahead of the defaults, thus removing the need for the two exceptions. But I will need to check that next.

The new MR does not change any existing functionality, but it does add --verbose and --debug options, and --input and --suffix options to allow easier local re-running without messing up the projects real composer.json and not overwriting the input.

The characteristic of placing the project's values after the default is due to the way merge_deep_array is written. The keys for the first array define the order, whereas the values of the second array take precedence and overwrite the first array. This means that by default, the output order is not preserved from what it is in the projects file. See attached file compare of projects original on the left, vs output on the right.

I've just been looking a merge_deep and merge_deep_array and I think the merge takes the later value and overwrites any already merged value with that key. So the order is correct for the top-level items, and if we switched it there would be incorrect values, say for top-level keys like 'description'. The problem we are trying to solve in the above change is not really a merge when the values overright, but simply an ordering of the values in the array 'installer-paths'. I don't know enough about the installer-paths array to see if what I've chnaged above is actually what @andrewbelcher wanted. But it has done no harm, so no need for any revert.

I've adde a --verbose flag and also a --suffix to allow easy repeated re-running locally, so we can see the outcome of changes. Will push that tomorrow.

There was a GTD failure in one of the phpunit jobs but was not related to curl. I re-ran and it passed.

Nice to see the new big clear message:

curl: (22) The requested URL returned error: 429
****************************************************************************************
[ERROR] Curl failure
DRUPALORG_CI_SERVER_URL=https://git.drupalcode.org
_CURL_TEMPLATES_REPO=project/gitlab_templates
_CURL_TEMPLATES_REF=default-ref
FILENAME=assets/phpstan.neon
FULL_URL=https://git.drupalcode.org/project/gitlab_templates/-/raw/default-ref/as...

Job ending with EXIT_CODE=22
****************************************************************************************

impossible to replicate (force the 429)

Yes :-) Does the log show any "retrying ..." line if there is a failure on first attempt? If so, maybe we can look out for that? Or even add some job to scan the log artifact file (if that's even possible).

RTBC anyway, then let's see if/what we get if we spot a 429

Yes I was wondering about that too. In principle it seems better to allow the project to override anything, not just the few exceptions we have discovered already. I may ask @moshe as he was probably involved in setting it up this way in the first instance, and there may be a design reason why it was done that way.

I'd be fine with you merging MR384 now, as we know that improvement is needed. But keep this issue open and I will make a second MR to investigate the full deep_merge.

Good to see you now have the correct .gitlab-ci.yml template, and that your pipelines now run.

There is the comment

# There is currently only compiled CSS files in the module.
  SKIP_STYLELINT: "1"

but the only .css file you have is this plain css/rift_cq.css so I don't see any reason to skip the stylelint job? Maybe those lines was left over from the other project. You can remove both of them and see what results the stylelint job gives.

Thanks for the hint @mondrake re #9 - #10. I have now sucessfully used this MR in a contrib pipeline. In my custom.xml I defined <testsuite name="only-default-time"></coce> then added <code>--types only-default-time --all --phpunit-configuration $PHPUNIT_CONFIGURATION_FILE_PATH in .gitlab-ci.yml and the specified subset of tests was run. This demonstrates that my specified custom.xml config must be active and that therefore we can use this mechanism in Gitlab Templates for Contib pipelines too.

Maybe this is a better example. The project defines:

 "extra": {
        "installer-paths": {
          "web/my/path": {
            "0": "type:drupal-module"
          }
        }
    }

Before, we got

"extra": {
        "installer-paths": {
            "web/core": [
                "type:drupal-core"
            ],
            "web/libraries/{$name}": [
                "type:drupal-library"
            ],
            "web/modules/contrib/{$name}": [
                "type:drupal-module"
            ],
...
           "web/my/path": [
                "type:drupal-module"
            ]
        },

but with this change we get

 "extra": {
        "installer-paths": {
            "web/my/path": [
                "type:drupal-module"
            ],
            "web/core": [
                "type:drupal-core"
            ],
            "web/libraries/{$name}": [
                "type:drupal-library"
            ],
            "web/modules/contrib/{$name}": [
                "type:drupal-module"
            ],
...

Just working on this now. To test, I have added the following in the test input composer.json

  "extra": {
        "installer-paths": {
          "web/core": {
            "0": "type:my-own-override"
          }
        }
    }

Without the change, the output composer.json shows

   "extra": {
        "installer-paths": {
            "web/core": [
                "type:drupal-core",
                "type:my-own-override"
            ],
            "web/libraries/{$name}": [
                "type:drupal-library"
            ],
...
 

With the change, we get the first two in the reverse order

  "extra": {
        "installer-paths": {
            "web/core": [
                "type:my-own-override",
                "type:drupal-core"
            ],
            "web/libraries/{$name}": [
                "type:drupal-library"
            ],
  ..

I have pushed the change, but can easily alter if this is not what is needed.

I echo @fjgarlin's words, regarding opening an issue in the project which needs this if that is possible. But it may not be as straight-forward as testing in a pipeline.

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

Nearly every call to \Drupal has been removed / replaced. The final ones are in SchedulerManager, which has 8 remaining.

2 x \Drupal::service('workbench_moderation.moderation_information')

/** @var \Drupal\workbench_moderation\ModerationInformationInterface $moderation_info */
$moderation_info = \Drupal::service('workbench_moderation.moderation_information');

line 426 and 644

2 x \Drupal::service('extension.list.module')

$optional_folder = \Drupal::service('extension.list.module')->getPath('scheduler') . '/config/optional';

line 1235, 1238

1 x \Drupal::service('config.storage')

/** @var \Drupal\Core\Config\StorageInterface $config_storage */
$config_storage = \Drupal::service('config.storage');

line 1249

2 x \Drupal::entityDefinitionUpdateManager()

$entityUpdateManager = \Drupal::entityDefinitionUpdateManager();

line 1181, 1297

1 x \Drupal::service('entity_display.repository')

/** @var \Drupal\Core\Entity\EntityDisplayRepositoryInterface $display_repository */
$display_repository = \Drupal::service('entity_display.repository');

line 1385

Merged. Thank you very much for doing this work, it is a good improvement.

I've merged in 2.x and resolved the conflict, so there's nothing more to do regarding #8.

Thanks for that. I've added the text to both of the paragraphs in the documentation MR328

jonathan1055 → created an issue.

Thanks for changing the image, nice that shellcheck is OK again.

For BC there is just one project using CORE_SECURITY_PREVIOUS_MINOR and this is because they have copied the entire includes/include.drupalci.main.yml file from Gitlab Templates to be their own .gitlab-ci.yml. That feels like the maintainer of rift_cq → has mis-understood the way that the template works.

The Check Code job failed with

$ # Bash standards. # collapsed multi-line command
/scripts-186820-5876141/step_script: line 235: shellcheck: command not found

After the install command apt-get update && apt-get install -y shellcheck we do see 404 Not Found [IP: 151.101.22.132 80]

The missingAction() function was not used, following the original MR, therefore deleted it in MR249

I have created https://github.com/pfrenssen/coder/pull/268

I can add test coverage if this is required.

Coder is being used to check some useful things in .yml files such as #2841649: Add sniff for _access: 'TRUE' in Drupal 8 routing.yml → and probably others. I think most php codesniffer rules will not trigger in .yml files, but some might, for example the one in this issue. Could you link to your pipeline job, or show the file(s) and sniff(s) that are incorrectly flagged. Then they could be excluded here too.

jonathan1055 → changed the visibility of the branch 3535411-exclude-some-sniffs-on-yml-files to hidden.

Actually we already cater for when the core actions do not exist. Can you give more of the log, to show where that error is coming from?

That's interesting. Some module must be deleting the core node_publish_action action. I see in your log there is entity:publish_action:node but I don't know if that is a replacement for node_publish_action or whether we could use that if the original action is not present.

1. Do you get the same error when you run cron via the site UI, and via a cron timed job?

2. Can you try to find out which 3rd-party module is doing this? Maybe start by listing your other modules and on a test site uninstall them one by one until the error goes away. Or alternatively, start with a clean dev site, and add your modules one at a time until the error starts happening.

I have also swapped round the display order of CORE_PREVIOUS_MINOR and CORE_SECURITY_PREVIOUS_MINOR so that the sequence is sort of getting older as the list goes down. Easier to understand 11.1.8 then 11.1.5 then 10.5.1. New output is

Current values of the variables             Variant where this is used
- CORE_STABLE:                  11.2.2      Current and max PHP
- CORE_SECURITY:                11.1.5      <unused>
- CORE_SUPPORTED:               11.2.x-dev  <unused>
- CORE_PREVIOUS_MINOR:          11.1.8      Previous Minor
- CORE_SECURITY_PREVIOUS_MINOR: 11.1.5      <unused>
- CORE_PREVIOUS_STABLE:         10.5.1      Previous Major
- CORE_MINOR:                   11.x-dev    <unused>
- CORE_NEXT_MINOR:              11.x-dev    Next Minor
- CORE_MAJOR_DEVELOPMENT:       <empty>     Next Major
- CORE_LEG_STABLE:              7.103       Drupal 7 tests

Yes, that's OK with no BC. You are right that there isn't really any case to solve. No CR but a simple message in slack would be good. Also I have changed the issue title to be specific, and we can make sure the commit message and changelog are very clear (those are more important as they persist, whereas a message in Slack may only be seen by a few)

I have added the (unrelated) missing word to .gitignore as per the discussion on #3532272-13: ESLint needs to ignore /vendor folder →

I didn't get that line into the MR on 📌 Exclude node_modules folder from artifacts Active but will add it into 📌 Is CORE_SECURITY_PREVIOUS_MINOR the right version for "previous minor" Active

I have this error in 8.x-2.6, but it's good that you have fixed it. Changing the title to be more helpful for search results.

Previously it was PHPStan version 1.12.27, used up to and including Core 11.1. Now we are running PHPStan version 2.1.27, in Core 11.2+ and it is more strict about ignorePaths which may not exist
https://git.drupalcode.org/project/scheduler/-/jobs/5844600#L36
Need to add (?) to indicate that the path is optional and may sometimes not exist.

Looks good, left a couple of suggestions in the MR.

Do we need to consider BC here? If a custom template has set the old variable CORE_SECURITY_PREVIOUS_MINOR to something, expecting that to be used in 'prevoius minor'. Not sure how we do that, as the old variable still has a value within this new set-up. So we can't check for whether it has a value. There is a way to reference variables from other sections, so could we give a warning if CORE_SECURITY_PREVIOUS_MINOR has a value other than the value defined in hidden-variables.yml ?

I can work on a MR and add test coverage if this is an agreed way forward.

jonathan1055 → created an issue.

📌 Fix 'Drupal.Commenting.InlineComment.InvalidEndChar' coding standard, pt 1 Active has been merged, so unpostponing this now.

Thanks for the merge. That's really useful information about the 'avoid commit conflicts' tag, I was unaware that there was a procedure to assist with that. Very useful.

I notice you have fixed a dependency injection, but we have 📌 Fix \Drupal calls should be avoided in classes, use dependency injection Active already, so I'd prefer if we keep this issue in scope. Was there a particular need for that to be fixed here? If not I will undo that, otherwise there will be a conflict.

I'm going to say this is fixed.

One thing that may be useful is if we could re-generate the variants page with the current values of those variables for 'default-ref' and for 'main' so that you could see exactly what is being tested. This could be done as a manually triggered job as part of fhe process when we bump the core versions. The Composer job logs always show this info, so it's in the jobs, but would also be useful in the docs too.

We have 📌 Documentation pages Active for continual improvements.

Thanks andrewbelcher for starting this issue, even though in fact there are not going to be any changes to the code, we could still improve our documentation if there are things that are missing or unclear, or just hard to find or not obvious. Could you take a look at the documentation site and see if there are things that could be improved in light of this thread. You have obviously taken some time to think this through, which we appreciate, so a new pair of eyes on the docs could be useful.

This looks interesting, thanks for working on it.
I've just merged 🐛 Unhandled Warning: Undefined array key "translatable" Active and would like to tag a new release soon, so maybe we can get this improvement in too?

Also fixed in 8.x-1.x

Thanks @c.lompole
The change that you suggest was already done in MR140 bu this has not been merged yet. I think also that we still need to check that $options['translatable'] is true, not just isset(), because isset() gives a positive even when $options['translatable'] is set to false. However, I have used your idea of isset() because this is preferrable to array_key_exists()

All tests pass, and the new test correctly fails in 'test-only changes'
https://git.drupalcode.org/project/scheduler/-/jobs/5828567#L584

Thanks everyone, merged and fixed.

When Gitlab Templates is updated to version 1.10.0 which will be soon, then all current PHPUnit jobs will be getting lots of these warnings as shown in the issue summary.

I have modified the title, to help developers find this issue, and also because symlinking is not necessarily the solution here (so is confusing if in the title)

Thanks. I re-ran the MR27 but of course this project does not use 'main' it has to use

  - project: $_GITLAB_TEMPLATES_REPO
    ref: $_GITLAB_TEMPLATES_REF

to enable it to function as desired when used in downstream pipelines. So I ran a pipeline via the issue fork UI setting _gitlab_templates_ref and _curl_templates_ref to 'main' and then we get the newly committed changes, and it works as required
https://git.drupalcode.org/issue/gitlab_templates_downstream-3513408/-/p...

Merged and fixed.

I have removed the temporary lines and created MR27 on #3513408-13: Add test coverage for stylelint job → for adding _COMPOSER_YARN_INSTALL: 0

I also made a readability improvement - the tests all run and pass the same, hope that was OK after you RTBC.

I've created MR27 so that we have permanent test coverage for the non-default _COMPOSER_YARN_INSTALL: 0. But it won't have any effect until 📌 Exclude node_modules folder from artifacts Active is merged, so maybe do that one first.

Also testing with _COMPOSER_YARN_INSTALL: 0 in the d10-plus branch here

Good, that confirms your comment in the MR that the "enable" is needed. We almost knew that was the case, but good to check. I have reverted that last commit. Now that MR26 is merged we get a downstream stylelint job too. Ready for review.

Thanks. Merged in d10-plus and cherry-picked into d10-theme and d9-basic. Both cherry-picks were clean and did not require any resolving of conflicts. Keeping the .gitlab-ci.yml file aligned where applicable means we make it easier for ourselves going forward merging things like this. But I didn't add this to main or d10-profile as the .css file is not so relevent on those branches. I presume that's OK?

OK, thanks. This issue is not too urgent, as it only affects the edge case when running cspell and also setting Drupal Core to 9.5. I think we're happy to wait for you to have time, if you want to do it and get some experience here, as it is good to have another contributor in the Gitlab Templates queue :-)

Tested via UI with new variable BEFORE_SCRIPT_ACTIONS set to stylelint-fail, and the job fails as required.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3513408/-/j...

Yes that's correct, those will be removed. I didn't make a comment about them in the MR because its more clear that they have to be removed. But the --verbose could get missed. But thanks for checking.

Would you like to add the new CSPELL_SHOW_PROGRESS variable as mentioned in #11? Happy to leave that for you, but likewise I can pick it up if you don't have time.

Ah! the file also needs to be in the .gitignore just like the other copied internal assets file. Shall I just add that into the MR on 📌 Exclude node_modules folder from artifacts Active to save making yet another new one here?

I have pushed the change to save then restore the current working directory. I also made the change to see if Stylelint fails when corepack enable is repeated in the script. But then I realised we do not yet have GTD coverage for Stylelint - it is the only remaining job left to add. We have had 📌 Add test coverage for stylelint job Active open for a while, so I have created a MR on that issue. Worth doing this properly now we have a direct need to test Stylelint.

jonathan1055 → changed the visibility of the branch 3513408-test-coverage-for-stylelint to hidden.

We need this now, because 📌 Exclude node_modules folder from artifacts Active makes changes to the Stylelint job, but we have no downstream test coverage of this.

I forgot to add the new .eslintignore file to the clean-up line when running the checks locally - new MR382

Test using MR381 with no changes, so installs node_modules as usual. Here is the full pipeline - all passed.
The composer log shows

Top 10 folders by size:
489	web/core
364	web/core/node_modules
118	web/core/node_modules/@ckeditor
66	web/core/modules
39	web/core/node_modules/ckeditor5
37	web/core/node_modules/ckeditor5/dist
30	web/core/node_modules/ckeditor5/dist/browser
18	web/core/node_modules/selenium-webdriver
17	web/modules/contrib
17	web/modules

The downloaded artifact zip is 142Mb. Unzipped 643MB

Test using MR381 with _COMPOSER_YARN_INSTALL: 0
Full pipeline all passed.
The composer log does not show web/core/node_modules. We see

Top 10 folders by size:
125	web/core
66	web/core/modules
17	web/modules/contrib
17	web/modules
16	web/core/assets
15	web/core/lib/Drupal
15	web/core/lib
15	web/core/assets/vendor
13	web/core/tests
13	web/core/assets/vendor/ckeditor5

The downloaded artifact zip is 57Mb. Unzipped 261MB, a significant decrease.

The eslint, stylelint and cspell jobs have this, which I don't understand, but is probably nothing to worry about:

➤ YN0007: │ @nightwatch/nightwatch-inspector@npm:1.0.1 must be built because it never has been before or the last one failed
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT 
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT > @nightwatch/nightwatch-inspector@1.0.1 build
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT > node preprocessExtension.js
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT 
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT 📂 Moving to src folder ...
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT 🚀 Creating .crx file ...
➤ YN0000: │ @nightwatch/nightwatch-inspector@npm:1.0.1 STDOUT ✅ .crx file created successfully!

Note that this only tests ESlint, Stylelint and CSpell, because Scheduler does not have any Nightwatch tests. I've made a couple of comments in the MR but it all looks good.

Yes RTBC I just left a comment in the MR on the description. Do we want to add "The range can be 1 - 32, with 8 as a good default" or something like that.

I made one more edit/suggestion in the doc page.

Do you think the GTD tests are enough? Shall I try this with Scheduler?

I have fixed the variable's description. If this is a blocker for the next release (i.e. updating default-ref) then we can go with this as-is. If we do follow up with the idea of our own variable then the standard one can be moved into the hidden-variables.yml file then.

Back to RTBC if you want to merge and do the release.

In reply to #3
@fjgarlin I will add you back as maintainer of GTD. But strangely I just tried and cannot trigger the downstream pipelines either. Just get the red "An error occurred while making the request."

In reply to #5 and #6

If we were able to do things like skip eslint & stylelint when JS & CSS aren’t changed in a MR, this would be a great approach. That should be a net reduction in compute.

We have 📌 Only run linting jobs if the files changed make sense for the job Active which sounds like what you want.

I was thinking we could create our own variable say _PHPUNIT_DEPRECATIONS which we can allow to have multiple values, to cater for future scenarios, eg. default 0 which would set PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION=0, 1 would set PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION=1 but it could also control other deprecations. If we didn't do that now, are we making it harder to implement in future?

Maybe it is OK to create this real variable anyway, but should it be in .variables.yml or hidden-variables.yml ?

Regardless, this is NW because the variable description needs to be tidied.

Thanks, yes csslint is not run in GitLab CI, we have Stylelint instead. Thanks for the tidy-up.

Added link to summary

This is great news. I'll work on a solution to have that folder as a variable, which can hopefully be changed in the pipeline 'pages' script, so that 'pages:deploy' can use it.

Hey, that's really satisfying that it found two broken links. But of course! that job is only run on commit to main. If I had thought more about this I would have made a temporary change to run 'pages' in the mr to find these two errors before commit.

quietone → credited jonathan1055 → .

Closed the MR now that Core 11.2 has this change merged in #3497431-93: Deprecate TestDiscovery test file scanning, use PHPUnit API instead → . Thanks for opening this issue, it was a good thing to test.

In Gitlab Templates we have a follow-ups 🐛 The /composer directory is not symlinked Active and 📌 Introduce PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION Active

Done, was going to RTBC but better for you to check the updated help doc paragraph

Sorry, got delayed with other things. Will do it here.

The command line in run-local-scripts.sh now matches the command in our .gitlab-ci.yml which is a "good thing"

But I noticed that the 'check code' job is already green, so those prettier failures must have been fixed upstream or something.
https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/377...
Still it is good that we have this .eslintignore file anyway.

Both jobs are green now.

Which implies that the eslint failures that will be avoided in 🐛 ESLint needs to ignore /vendor folder Active have already been fixed externally.

jonathan1055 → created an issue.

In run-local-checks.sh we have catered for this by adding --ignore-pattern=vendor --ignore-pattern=node_modules so could do the same in the gitlab job. But I think it may be tidier to use a .eslintignore file, which can be active for both local and the pipeline job.

jonathan1055 → created an issue.

There is also the --site-dir public parameter in mkdocs. If this could be changed for MR runs, would that be possible? I think the 'pages:deloy' may be expecting 'public' because I did a quick test with a different directory. But is that something we can explore?