🇬🇧United Kingdom @jonathan1055

Yes, leaving the additions in the main module plugin is fine.
I am going to push a change to .gitlab-ci.yml to make the 'next minor' testing manual. So when done, you will need to pull that down to your local repo. Are you currently in the middle of making changes? Let me know when you have pushed, as I don't want to make more work for you.

I have added two sed to change the full path into just the name

Tested in API 2.x here

Ready for feedback. I have also tested in Scheduler adding both a phpcs.xml and phpcs.xml.dist to make sure the job copes with that (even though the second one is redundant and would be ignored in any case)

Would prefer the plugin for product variations to be stored in a third party module or is it a commonly used enough entity to be included here

It woud be good if it can be included in Scheduler, provided there is not too much overhead, as it fits neatly here. It would take more effort to be a separate third-party module. But is there any benefit in putting it within a submodule of Scheduler which only gets enabled when required? We do this for scheduler_rules_integration. It may make the code more messy, and might not warrant the effort. But just making the suggestion in case you like the idea.

running tests locally ... missing dependencies

Is this why you added 'language', 'locale' and 'config' to SchedulerBrowserTestBase here ?

Yes we need to ensure that existing jobs run OK. The link you gave had 184 instances, but many of those are in scripts in composer.json or other build framework .yml files, or in vendor files. Those will not impact the pipeline job at all. I tried to see how many are in actual phpcs.xml(.dist) files and it is about 80 https://git.drupalcode.org/search?group_id=2&language%5B%5D=XML&page=4&r...

So maybe in the phpcs job after determining that we are going to use the project's own config, we can sed to replace the hard-coded path with the name of the rule. I will try that, and will use the API merge request to test it.

Amazing timing! I was looking at the pipeline after @dww added to the 'upgrade status' documentation page. It ran at 17:27 but was still reporting 11.1.2 as the latest
https://git.drupalcode.org/issue/gitlab_templates-3473000/-/pipelines/42...
I then rebased MR328 and it ran green

Drupal 11.1.3 was created at 17:24, or so it says on this page
https://www.drupal.org/project/drupal/releases/11.1.3 →
So 11.1.3 was actually released then, but the job did not report it as being available yet. I guess there is a delay in some part of the packaging & release process. Interesting.

Ha ha! Only yesterday I accidentally opened 📌 Bump core stable versions to 11.1.2 and 10.4.2 Active but the pipeline I was looking at was on an out-of-date MR. Then the very next day, there is a real one.

Hi @tcrawford,
This is excellent work, thank you. It seems like you know what to do, and are making good progress. In case it helps, have you read the new Scheduler documentaion regarding the plugins

Also, to speed up test times and save resources, you could comment out the other entity types from dataStandardEntityTypes(). Then you'll get quicker jobs, shorter logs and be able to identify the problem tests more easily. Let me know if I can help.

jonathan1055 → changed the visibility of the branch 3507840-core-11.1.2 to hidden.

jonathan1055 → changed the visibility of the branch 3507840-core-11.1.2 to hidden.

Ignore this issue. The MR pipeline was run without a latest rebase.

jonathan1055 → created an issue.

I have discovered the cause of the error. The API project has it's own phpcs.xml file, but the rule is defined as
<rule ref="vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml"/>
see https://git.drupalcode.org/project/api/-/blob/2.x/phpcs.xml.dist#L4
It is a hard-coded path relative to the project top-level directory. So when we run the job within the project directory it does not find this, and we get

$CI_PROJECT_DIR/vendor/bin/phpcs -e || true
ERROR: Referenced sniff "vendor/drupal/coder/coder_sniffer/Drupal/ruleset.xml" does not exist

But when the command is run at the original top-level folder (as happens in the added after_script, it works as before.
Here is the job that shows all this https://git.drupalcode.org/issue/api-3391946/-/jobs/4394940

A better way to specify the rules is by name, which we now do in this MR, see this change

If the relative hard-coded path is widely used maybe we can fix that in this job, or there are other solutions I'm sure. But at least we know now what the problem is.

That's what I thought, although that would be surprising. I updated API MR22 and added

phpcs:
  after_script:
    - $CI_PROJECT_DIR/vendor/bin/phpcs -e 
    - $CI_PROJECT_DIR/vendor/bin/phpcs -e | grep -E 'contains.*sniffs'
    - $CI_PROJECT_DIR/vendor/bin/phpcs -e | grep -E 'contains.*sniffs' || true

and this worked fine, exactly as intended. But oddly it did fail in the actual phpcs script, the same as when you ran it via downstream. So at least I can replicate the problem. I will add || true to the real MR, and also remove the 's' from 'sniffs' in the pattern, as a project could conceiveably have one sniff (although extremely unlikely). But that is not the cause of the problem here.

OK. I will test ✨ Integrate Gitlab CI template for D7 Active

I thought you would like the bonus in 11.

Just re-read the comment in MR38, it was about not having a link to the file. There is nothing shown in the 'filename' column. I don't know how to add that, but it would be a separate follow-up anyway.

I've removed the debug code and the downstream override variables. Also moved the "There are no PHPCS coding standards errors or warnings" to the end of the script. Downstream pipelines all OK.

MR325 is ready for review and feedback. Here is a summary of the changes:

1. Switch to the drupal project folder to run the phpcs script, therefore using . for the files to check, not $_WEB_ROOT/modules/custom or $_WEB_ROOT/sites/all/modules/custom
2. check for the existence of either phpcs.xml or phpcs.xml.dist in the project folder, before getting the /assets/phpcs.xml file. Previously we only checked for phpcs.xml.dist and this worked fine because if the project had their own phpcs.xml it would take precedence over the curl'd phpcs.xml.dist. But doing the curl when not needed was not only wasting resources but also giving confusing info in the log
3. Improve the message about which phpcs.xml file is being used
4. Show info on the package versions using the simpler composer show | grep which gives better formatted output compared to composer show | awk {print $1 " " $2}
5. Add --basepath=$DRUPAL_PROJECT_FOLDER to the options. D10 did not have this option before. D7 did have it but was set incorrectly starting with $_WEB_ROOT so did not work properly.
6. Add --colors and --report-width=120 options to $_PHPCS_EXTRA if they are not already specified in that variable
7. Show the name of the standard being used and how many sniffs it contains
8. Echo "executing " to show the exact full command that is being run
9. Exit at the end of the job, not immediately on failure of phpcs
10. Check the return code properly and display "There are no PHPCS coding standards errors or warnings" if all is OK.
11. As a bonus - due to the new $DRUPAL_PROJECT_FOLDER variable holding the different path for Drupal 7 vs Drupal 10, the complete phpcs job definition can now be identical in the D10 and D7 files.

Before, to show the problems

d7-basic with forced failure BEFORE_SCRIPT_ACTIONS = phpcs-fail
log show long path FILE: ...ules/custom/gitlab_templates_downstream/gitlab_templates_downstream.module
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/43...
junit.xml has long paths
tests tab has long paths

d7-composer, clean
log shows long path
junit.xml has long path
test tab has long path

d9-basic with forced failure, same faults as above
log, junit and tests tab

d10-plus, clean
log, junit and tests tab

After, in downstream jobs

forced failure via BEFORE_SCRIPT_ACTIONS: 'phpcs-fail'
d7-basic
Log has short file path
junit.xml has short path
Tests tab has short path

d7-composer, clean
log has short paths, as do junit.xml and tests tab

d9-basic with forced failure
log has short path, same with junit.xmland test tab

d10-plus, clean
log, junit.xml and tests tab

The complete pipeline for MR325 is https://git.drupalcode.org/issue/gitlab_templates-3380694/-/pipelines/42...

I have left the original MR38 open as it has some comments in, so just need to check that.

jonathan1055 → created an issue.

jonathan1055 → created an issue.

I have fixed the long lines in README.md and the rendered file looks fine.
https://git.drupalcode.org/issue/faker-3506099/-/blob/3506099-fix-phpcs/...

I have added a phpcs.xml file to include all sniffs in DrupalPractice and also to check .md files. The job shows

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
SOURCE                                                           COUNT
----------------------------------------------------------------------
Drupal.Commenting.ClassComment.Short                             15
Drupal.Files.TxtFileLineLength.TooLong                           3
DrupalPractice.Objects.GlobalDrupal.GlobalDrupal                 2
----------------------------------------------------------------------
A TOTAL OF 20 SNIFF VIOLATIONS WERE FOUND IN 3 SOURCES
----------------------------------------------------------------------

[The issue summary update is just to make it more readable, I re-ran on 3.x with --report-widht=90 and a --basepath option]

jonathan1055 → created an issue.

jonathan1055 → created an issue.

From slack thread

you could even push a tag in one of the branches in the fork. if I’m right, CI will run, and with the MR, it should not (within the fork)

That's a good idea, and it worked. I pushed tag 0.0.3 to the 3503851-tags branch and a pipeline was triggered.
I then committed a change to use this MR, pushed tag 0.0.4 - and no pipeline was triggered.

Here are the commits and here are the tags with pipeline on 0.0.3 but not on 0.0.4

This is not precisely the scenario that @jurgenhass reported, but I think this does demonstrate the same behavior, and therefore shows that MR330 fixes the problem, so this is RTBC

I have tried to test this in GTD but not yet sucessfully:

1. The d7-basic branch has two tags (although only 0.0.2 shows in the commit list)
2. I created a new branch named 3503851-tags in the old issue 📌 Add custom variables to set before_script commands in upstream project Active
3. I created MR4 and a pipeline ran immediately (as expected)
4. It needed a change to a file before the "2 commits behind target branch" message was shown. The second pipeline was triggered (as expected)
5. To demonstrate the problem (before testing against MR330) I rebased MR4 hoping to see at least one extra pipeline due to the new tag. But there was only one new pipeline, due to the rebase, no extras

Is this because the branch is not the default branch? The change in MR330 does not look like that is the problem. Until we can replicate the old behavior I cannot say that this change solves the problem.

https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/tags

Cherry-picked and pushed to the other three branches. Pipelines triggered on all of them, see attached.

That worked, the second push triggered a pipeline as required.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

I am going to merge this, just onto the d7-basic branch for now, so we can verify that it works as intended. Then I can cherry-pick to the other branches when we know.

As a bonus, I have corrected the spelling error (UK -> US) so that job is green now.

I'm happy to help with setting up the phpcs.xml file if you want, depending on your views of the two points in #5

But I will let someone else write the proper short descriptions of the classes, someone who knows about this project. Over to you ...

The reason for the reduced number of faults compared to the output shown in the issue summary are due to two things:

1. The DrupalPractice standard is not included by default in the gitlab pipeline phpcs job. Only the Drupal set of rules is used. If you would like the pipeline job to check against best practices, then the way to do this is to have a phpcs.xml.dist file in the project folder, and to specify both sets of standards. Here is an example
2. The reason that the long lines in README are not reported is that .md files are not included by default. Again this can be changed by adding 'md' as an extension, see this example

Starting point, the pipeline phpcs summary shows

--------------------------------------------------------------------------------
PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
--------------------------------------------------------------------------------
    SOURCE                                                                 COUNT
--------------------------------------------------------------------------------
[ ] Drupal.Commenting.ClassComment.Short                                   15
[x] SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue.Null  7
[x] SlevomatCodingStandard.PHP.ShortList.LongListUsed                      1
--------------------------------------------------------------------------------
A TOTAL OF 23 SNIFF VIOLATIONS WERE FOUND IN 3 SOURCES

Initial push on MR4 now has

----------------------------------------------------------------------
SOURCE                                                           COUNT
----------------------------------------------------------------------
Drupal.Commenting.ClassComment.Short                             15
----------------------------------------------------------------------
A TOTAL OF 15 SNIFF VIOLATIONS WERE FOUND IN 1 SOURCE
----------------------------------------------------------------------

I will work on this more tomorrow.

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

I can't seem to disable the current composer job

This is because the linting and code-quality jobs all "need" the current composer job. But that is working fine, now that you have made a few changes to composer.json

If the project had phpunit tests then the core version requirements in .info.yml would be more important, and would stop the tests running at D11. But I think this is fine as-is. I also agree with you that the core version is not needed in composer.json, and infact I read some documentation saying that it is best not to have it duplicated, unless you really need it for external reasons.

So I think MR3 is ready for review and then merging, then we can work on fixing the other jobs in follow-up issues. It will be good to get the initial .gitlab-ci.yml committed.

I have checked the three changes in D7 and they work as expected. The .info file has

test_dependencies[] = date
test_dependencies[] = rules

and to force the 'install dependencies from .info' I deleted the project's composer.json, but oddly it could not install the date module. The error was

drush dl date
There are no stable releases for project date.                       [warning]
Choose one of the available releases for date:
 [0]  :  Cancel                                         
 [1]  :  7.x-3.x-dev     -  2023-Feb-01  -  Development 
 [2]  :  7.x-3.0-alpha2  -  2022-Sep-07  -  Recommended 
 [3]  :  7.x-2.x-dev     -  2023-Feb-01  -  Development 
 [4]  :  7.x-2.8         -  2014-Jul-29  -  Security    
 [5]  :  7.x-1.x-dev     -  2017-Mar-28  -  Development
Cancelled.         

But it runs fine with a composer.json containing

"require-dev": {
  "drupal/date": "*",
  "drupal/rules": "*"

But it not the fault of the changes in this MR, hence putting it back to RTBC as before.

All D10 changes have been checked - see links in issue summary.

Yes, I was going to ask if we want to do calculations on the new variable, and/or use .. . I can work on those in the follow, as this issue already has enough of the basic simple changes.

I have updated the issue summary with the actual job that shows each change. No all are covered yet, so I have put this back to NR until I have done the rest. But will return it to RTBC if nothing else is found that requires fixing.

jonathan1055 → changed the visibility of the branch 3.0.x to hidden.

The 8.x-1.x pipeline now passes and is fully green.

I have found a small error in the commit from this MR, there is a double $$ in the 'test-only chnages'
The job still runs fine, it just shows an error in the log. This is no worse than before, because if anyone had changed the working directory in a test-only changes before_script they would already have had to reset it.

I have fixed it in MR331 in 📌 Create environment variable for the project's own folder Active and I think this will be fine to wait and do it here. I don't think we need to fix it immediately and make another default release, because it won't be long until this is done.

It would be nice to get a phpunit test job in each of the GTD branches, then we can have a BEFORE_SCRIPT_ACTIONS key to somehow trigger a 'test-only changes' job. That might need some ingenuity as that job only works on MRs. But we can solve that when we get there.

Thank you for making the 1.7.2 release. A nice set of issues and improvements, plus the core version bump of course.

This will allow me to finish 📌 [ignore] Testing Gitlab Templates MRs Active

Another thing to check is whether all folders are being checked. I am sure this used to be the case, but this recent cspell job only checked 7 files at the top level. It hasCSPELL_SEARCH=$([[ $_CSPELL_EXTRA =~ "--dot" ]] && echo "{**,.**}" || echo "**") and the result is ** as expected. The src and tests folders are shown in the potential list of files but no files in them are checked.

https://git.drupalcode.org/issue/scheduler_content_moderation_integratio...

The checking/debug in composer log shows:

$ BR=d7-basic
$ if [[ $BR =~ ^(d7-basic|d7-composer|d9-basic|d10-plus)$ ]]; then echo "BR $BR matches"; fi
BR d7-basic matches
$ BR=other-d7-basic
$ if [[ $BR =~ ^(d7-basic|d7-composer|d9-basic|d10-plus)$ ]]; then echo "BR $BR matches"; else echo "BR $BR does not match"; fi
BR other-d7-basic does not match
$ BR=d7-basic-more
$ if [[ $BR =~ ^(d7-basic|d7-composer|d9-basic|d10-plus)$ ]]; then echo "BR $BR matches"; else echo "BR $BR does not match"; fi
BR d7-basic-more does not match

So I think the actual rule should work
if: $CI_COMMIT_BRANCH =~ /^(d7-basic|d7-composer|d9-basic|d10-plus)$/ && $CI_PROJECT_ROOT_NAMESPACE == "project"

But we cannot be sure until this is merged. That has to wait until the workflow commit in ✨ Change workflow: into a re-usable reference that can be extended Active has reached 'default-ref'. These downstream projects cannot have a fixed ref 'main', otherwise they won't be testing the upstream MR branch.

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

Merged MR3 into d7-basic and cherry-picked onto d7-composer.
Thanks for you help and reviews.

The d7 version works. Tested here using downstream pipeline from ✨ Add basepath parameter to PHP Code Sniffer Active and setting

'→ GTD D7 Basic':
    # @todo reset this back to default repo and branch when issue 3503851 is merged.
    project: issue/gitlab_templates_downstream-3503851
    # cspell:disable-next-line
    branch: 3503851-customisable-before-script-d7
  variables:
    # Force phpcs to fail, to demonstrate the xml report and artifacts.
    BEFORE_SCRIPT_ACTIONS: 'phpcs-fail'

https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...

I think the downstream API test failures are unrelated to this change. The tests run but one fails with

Drupal\Tests\api\Functional\LinkCodeTest::testApiLinkCode
    Linking to a PHP function
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'strpos'
    +'strpos'

I have pushed changes to composer-lint find, composer-base and phpunit when concurrent=0. Need to test these three.

The remaining places are:
D10
eslint makes links in the folder above $CI_PROJECT_NAME so that has to stay as-is

phpunit concurrent=1 uses --directory modules/custom/$CI_PROJECT_NAME - will need to test if this can be changed to absolute

upgrade status has

sed -i "s#modules\\\/custom\\\/$CI_PROJECT_NAME\\\/##" upgrade-status-analysis.json

phpcs

vendor/bin/phpcs -s $_WEB_ROOT/modules/custom --report-junit=junit.xml ...

This will all change in ✨ Add basepath parameter to PHP Code Sniffer Active so I was leaving it untouched until the full testing in that issue

D7 phpunit has

sudo SYMFONY_DEPRECATIONS_HELPER="$SYMFONY_DEPRECATIONS_HELPER" MINK_DRIVER_ARGS_WEBDRIVER="$MINK_DRIVER_ARGS_WEBDRIVER" -u www-data php $_WEB_ROOT/scripts/run-tests.sh --color --concurrency $_CONCURRENCY_THREADS --url "$SIMPLETEST_BASE_URL" --verbose --fail-only --xml "$BROWSERTEST_OUTPUT_DIRECTORY" --directory sites/all/modules/custom $_PHPUNIT_EXTRA || EXIT_CODE=$?

so that will need testing (not changed yet)

Yes you are right. I thought that the ones remaining were only where we had to use modules/custom without the project name. But there are some instances where the full path is still used, but it is relative, i.e. starts with $_WEB_ROOT, maybe that's why I missed them on the global search. I will change those if possible. Will need to test the find command which has a relative path.

Full pipeline test https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/420834

jonathan1055 → changed the visibility of the branch mr327-3503966-cd-pwd-in-script to hidden.

I've updated the MR and added a before_script & after_script paragraph in 'customizations' with examples.

Will do full testing using Scheduler.

I'm pleased to help.

The link you gave is to the dictionaries that Core uses. But for contrib, Gitlab Templates expands that list to use a lot more good dictionaries which contrib projects can make use of, to avoid too many custom additions.

Good thinking, yes starting it with DRUPAL_PROJECT_ makes sense. Let's not use _DIR because that is too similar to CI_PROJECT_DIR. I have a slight preference for DRUPAL_PROJECT_FOLDER so will do that, and we can see how it reads in the code. But if you have a good reason for DRUPAL_PROJECT_PATH I can easily change it.

Also noting that this will need documenting somewhere.

The phpstan and eslint GTD jobs now show

Executing "step_script" stage of the job script 00:07
$ cd $PROJECT_FOLDER && pwd
/builds/project/gitlab_templates_downstream/web/modules/custom/gitlab_templates_downstream

Ready for review and feedback (and more testing in other projects)

MR331 creates a new variable named $PROJECT_FOLDER but this can be changed if we think of a better name.

I was going to suggest this enhancement anyway, as it is helpful for anyone who is writing custom before_script and after_script. But another major advantage of having a variable, is that for D7 branches the path is slightly different -
$CI_PROJECT_DIR/$_WEB_ROOT/sites/all/modules/custom
So when cherry-picking commits across the branches in Gitlab Templates Downstream → having a variable to hold this path would mean we keep more of the template file aligned, rather than having to edit the path.

jonathan1055 → created an issue.

Thanks for the review. I have merged MR1 into d9-basic and cherry-picked onto d10-plus

I'm going to do a MR for the first Drupal 7 branch, as that will require editting due to no cspell job, so would be good to make sure I've got that correct before committing.

Run as a downstream pipeline with the following

'→ GTD D9 Basic':
  variables:
    # Force phpcs to fail, to demonstrate the xml report and artifacts.
    BEFORE_SCRIPT_ACTIONS: 'phpcs-fail'

and it fails as required
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...

The changes in this MR on the d9-basic branch will also be needed in the other branches. Once it is merged, is the best way just to do a simple git cherry-pick? The branches are mostly aligned, so that should not be a problem. It will encourage us to keep things the same over the branches where possible.

Tested here - plain test, no variables
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

UI with

BEFORE_SCRIPT_ACTIONS = other-thing phpcs-fail
_GITLAB_TEMPLATES_REF = main

https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...
phpcs job fails as required

Added actions for cspell. Plain pipeline
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

UI with

BEFORE_SCRIPT_ACTIONS = phpcs-fail cspell-use-madeupword
_GITLAB_TEMPLATES_REF = main

both jobs fail as required
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...

UI with

BEFORE_SCRIPT_ACTIONS =  cspell-use-madeupword cspell-project-words something-else

https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...
No failures, green as expected

This is ready for review. I am also going to test it as a downstream project in MR325 on ✨ Add basepath parameter to PHP Code Sniffer Active

Also I note that build.env is being checked. This is not a file that is under the control of the project, so we should always skip this.

The empty artifact file _cspell_updated_project_words.txt is being checked on the second run, and there is no need to include this either, as it is just confusing in the log.

Whilst making these changes to cspell, we can also use this issue to fix the error that when verbose -v is added in $_CSPELL_EXTRA this is also used in the second cspell command when we create the words file. So we get the verbose text saved to the $WORDS_FILE artifact which is unhelpful, and also gives the wrong count for number of words mis-spelled. The verbose output is not needed twice, so it should be easy to remove '-v' from $_CSPELL_EXTRA before calling the command for the second time.

jonathan1055 → created an issue.

Updated title to reflect the changes. I will resume working on this when 📌 Add custom variables to set before_script commands in upstream project Active is done, as the downstream projects will be used to test these changes.

I have moved the phpcs config file to a new hidden .gitlab/ folder, as you suggested. I would like to suggest two more ideas which I have committed and should make this better:

1. I can envisage scenarios where more than one thing needs to be done in the before_script when we are testing MRs, for example when testing a change in cspell we may want an action 'use-unrecognized-word' which causes the job to fail, then we run the test again with 'use-unrecognized-word' and 'update-project-dictionary' to demonstrate that this now fixes the job. It would therefore be useful to allow PHPCS_BEFORE_SCRIPT to contain a list of actions, not just one, to avoid duplication in the GTD template. It would be a simple pattern match if [[ $PHPCS_BEFORE_SCRIPT =~ "make-phpcs-fail" ]] instead of checking exact equality.
2. Having a list of actions would mean that there is no real need to have a separate "before script" variable for each job. Instead of having $PHPCS_BEFORE_SCRIPT, $CPSELL_BEFORE_SCRIPT, $ESLINT_BEFORE_SCRIPT, etc we can have just one variable, say $BEFORE_SCRIPT_ACTIONS, which holds any and all actions we want. This will make it easier to copy/paste when setting up the template and the steps in GTD. There is only one variable to set and to check, so we are less likely to make mistakes on either side of the link between the two projects.

I have pushed the changes, and will add an exampls for CSpell and run some tests.

Happy to give you co-maintainership too if you're interested?

Thank you for the offer. But I will just help out here, for now, as I know nothing about the module yet.

... we can still support D11 in either 3.0.x or 3.1.x depending on the changes.

OK, that's good. We can work on that. I see that it is only the .info.yml file that sets core_version_requirement: ^9.3 || ^10. There is no

  "require": {
    "drupal/core": "^9.3 || ^10",

in composer.json. I'm not sure, but is it good practice to have the core version specified in both places?

The composer job shows this conflict.

Your requirements could not be resolved to an installable set of packages.
  Problem 1
    - Root composer.json requires drupal/core-recommended 11.1.1 -> satisfiable by drupal/core-recommended[11.1.1].
    - Root composer.json requires roave/security-advisories dev-master -> satisfiable by roave/security-advisories[dev-master].
    - drupal/core-recommended 11.1.1 requires twig/twig ~v3.16.0 -> satisfiable by twig/twig[v3.16.0].
    - roave/security-advisories dev-master conflicts with twig/twig v3.16.0.

But I now see from the project page that 3.0.0-beta2 does not claim to be compatible with Drupal 11, so that probably explains it. I'm not familiar with Faker, but I came here due to my work on both Devel Generate and Gitlab Templates, and was thinking I could help here.

I can easily modify the gitlab template so that it runs the code quality tests at Drupal core 10, not 11, if that is the plan for the 3.x branch. I see you also have 🌱 [META] Plan for Faker 4.0.0 Active so maybe your plan is to have that branch compatible with D11.

I'm happy to help here, and will wait for your guidance on what the plans are for core compatibility with 3.x

I have created MR3 and the pipeline runs.

When this is merged we can work on the failing jobs, but first we need to get the composer job to run sucessfully.

jonathan1055 → created an issue.

There are still words that do not need to be specified. I have re-based MR37 so it is mergeable now.

It was also my fault in #8 as I showed my earlier idea which I then went on to reject, so I'm not surprised I confused you. The script idea could be useful in more complex requirements, which can investigate if/when that need arises.

I pushed the change yesterday to MR1 and it works. I just did it in the template step, not a separate script.

Pipeline with no variable, using 'default-ref'
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

Triggered from upstream MR with PHPCS_BEFORE_SCRIPT = make-phpcs-fail
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...

Pipeiline via UI with
_GITLAB_TEMPLATES_REPO = issue/gitlab_templates-3380694
_GITLAB_TEMPLATES_REF = 3380694-run-phpcs-in-project-folder
PHPCS_BEFORE_SCRIPT = make-phpcs-fail
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503851/-/p...

This is quite simple, and we only have one use-case right now. But we can add 'make-phpunit-fail', 'make-phpstan-fail' etc when we need them. Do we still need to investigate the separate script idea?

I've put back the working example of this into the MR. Essentially it is

phpcs:
  before_script:
    - cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME && pwd
    - |
      if [[ $PHPCS_BEFORE_SCRIPT != "" ]]; then
        echo "Executing command PHPCS_BEFORE_SCRIPT=$PHPCS_BEFORE_SCRIPT"
        $PHPCS_BEFORE_SCRIPT
      fi

This does work for this scenario, but it is not very generic, as it assumes that the command(s) contained in $PHPCS_BEFORE_SCRIPT should always be run in $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME. It also divides the active functionality between the upsteram (where the command is set) and the downstream (which changes the directory). So I don't think this is the best way forward.

Your idea of scripts in the downstream project sounds like it would work, but my original idea was that the upstream project could define any commands it needed, and we'd not have to commit changes to downstream. But maybe we should have scripts (or just re-usable code references) each designed to achieve a specific thing. It could be called something like "make-phpcs-fail" which is all that we actually want at this time. This would be clearer to understand in the upstream project trigger definition. It shows what we want, and does not have to be concerned with how that is actually donw. Then instead of $PHPCS_BEFORE_SCRIPT being a command or a 1/0 flag it could be the name of that re-usable reference in the downstream project.

Merged. Thank you all for your contributions here.

I have now released 2.2.1 with this fix
https://www.drupal.org/project/scheduler/releases/2.2.1 →

I've removed the temp branch code and the @todo and made the other requested changes, so this is ready for review.

Thank you for the quick review and merge.

Thanks @dieterholvoet. With that info I did manage to replicate the error, but it was only for the duration of the database update. The message showed probably when Scheduler was updated earlier in the list before SCMI. But after the update was complete there was no lingering problem and the site functioned fine. However, it seems there is no downside to having the aliases declared twice, so I'm going to commit this then release 2.2.1

Thanks for working on this. I'm using address and commerce as 3rd-party testing dependecies in Scheduler (because we provide a plugin to allow scheduling of products). Testing on MAX_PHP, PHP 8.4 I still get the error

Deprecated: address_post_update_default_widget_wrapper(): Implicitly marking parameter $sandbox as nullable is deprecated, the explicit nullable type must be used instead in web/modules/contrib/address/address.post_update.php on line 16

The composer log shows Address 2.0.3 so I thought the fixes in this issue would be included in that release.

The pipeline is https://git.drupalcode.org/project/scheduler_content_moderation_integrat...

To make sure, I have just added cd .. && echo "directory changed" && pwd to every before_script: in this test.
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/415668
The logs show the change, but everything ran OK.

This improvement will help with the custom before_script work, that's why I switched to progressing this issue first.

jonathan1055 → changed the visibility of the branch 3502119-D10-for-2.x-branch to hidden.

I'm using this issue fork to test general pipeline changes. They do not need separate MRs, we can test in a branch, then pick the commits. I have created a 3.x test branch

This issue can be left 'fixed' and then will become 'closed (fixed)' but I can continue to use the issue forks.

I have now added the plain workflow syntax to MR2

1. A pipeline ran on that push
2. Edit and commit via web to set max_php:0 and the pipeline was triggered
3. Added the workflow into the 'main' branch. No pipeline was triggered. Usefully the pipeline validation did run, as shown by the first red error, because the reference does not exist in 'default-ref'. Second commit to use MR326 - the validation passes, but no pipeline triggered (as expected)

I also made a temporary change to MR326 so that the downstream d7-basic runs the same issue branch I've been testing above. Here is that downstream pipeline. All OK.

This testing, plus the customised extra rule in #6, gives me confidence that this is working as expected. Ready for feedback.

Before adding adding

workflow:
  rules:
    - !reference [ .default-workflow ]

to this branch, I have done some testing to check the new workflow syntax when it is used only in MR236.

1. This pipeline via UI, which shows that if: $CI_PIPELINE_SOURCE == "web" is working
2. Created MR2 and a pipeline ran immediatley as required, showing that if: $CI_MERGE_REQUEST_IID and if: $CI_PIPELINE_SOURCE == 'merge_request_event' work.
3. I made a commit to the default 'main' branch in that issue fork and no pipeline was triggered

Next, to add that code and repeat these tests.

You are welcome, and thank you for bringing the problem to our attention. We are working on a solution to detect when the project has a directory matching _$WEB_ROOT and give an informative message in the log.

Thanks @hchonov for the quick turn-around. We got the issue to the correct project in the end :-)

I've tested this MR on Scheduler
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/415134
The cd && pwd all seem fine in the log. It has all the jobs except Nightwatch and Upgrade Status.

jonathan1055 → changed the visibility of the branch 3473000-doc-update-7 to hidden.

I've pushed the initial changes

For info, I checked and none of the following need any specific directory, but I still prefer to have the cd as the very first line in the script:
*check-composer-end-code
*show-environment-variables
*setup-webserver
*simpletest-db
*show-context

*amend-core-requirements-next-major already saves and restores the current directory.

Summary of changes

Added default cd $CI_PROJECT_DIR && pwd to

• .composer-base
• pages:
• composer-lint:
• phpcs:
• upgrade status:
• cspell
• .phpunit-base
• test-only changes:

Changed from relative to absolute

• stylelint: has relative cd $_WEB_ROOT so made this absolute $CI_PROJECT_DIR/$_WEB_ROOT
• .nightwatch-base has relative cd $_WEB_ROOT so changed this to absolute $CI_PROJECT_DIR/$_WEB_ROOT

Not changed (except for adding pwd)

• .phpstan-base already has cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME
• eslint: already has cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME

In main-d7 did the same - added to default to .composer-base, phpcs, .phpunit-base, test-only changes

Ready for testing and feedback

I've c reated GTD 📌 [ignore] Testing Gitlab Templates MRs Active . First commit on the branch based on d7-basic just added the repo and ref. No pipeline of course. Do we want to test anything on that branch before I add the three workflow lines from #7 ?

jonathan1055 → created an issue.

Here is a test using this MR on Scheduler.
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/414840
The pipeline did not run automatically until I added

workflow:
  rules:
    - !reference [ .default-workflow ]
    - if: $CI_COMMIT_BRANCH == "mr326-3504083-workflow-reference"

So in principle the syntax looks good, because if it works for some of the rules it should be the same for all. But some other scenarios would be nice, just to make sure.

The syntax is easy, I've already got it working. But yes we need to test it in several of the scenarios.

I have created the full SCMI 2.0.0 release →

Scheduler 2.2.1 will be coming soon.

Done.

The multiple deprecation warnings Implicitly marking parameter as nullable is deprecated, the explicit nullable type must be used instead in the phpunit tests are for 3rd-party modules such as Address, Entity and Commerce that we use in testing. The log is very long but I could not see any deprecations for SCMI functions.

Even though we are not testing 2.x with Drupal 11 we do need to be able to test it at PHP8.4. This can be done with the variable, and will need the core version set to Drupal 10 not 11.

If the command is just a rename, then it executes in the top-level $CI_PROJECT_DIR folder. The real file is renamed, because we see renamed 'phpcs-not-active.xml' -> 'phpcs.xml' in the log. But the symlink (which is what is being used by phpcs) is not renamed, and we see

lrwxrwxrwx 1 root root 48 Feb  2 16:59 gitlab_templates_downstream.info.yml -> ../../../../gitlab_templates_downstream.info.yml
lrwxrwxrwx 1 root root 46 Feb  2 16:59 gitlab_templates_downstream.module -> ../../../../gitlab_templates_downstream.module
lrwxrwxrwx 1 root root 32 Feb  2 16:59 phpcs-not-active.xml -> ../../../../phpcs-not-active.xml

in the log. Then the job still downloads the default assets/phpcs.xml which is not what we want.
/builds/issue/gitlab_templates_downstream-3503851/web/modules/custom/gitlab_templates_downstream-3503851

I tried to change directory within the command string, to effectively have cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME; mv -v phpcs-not-active.xml phpcs.xml but I could not find a way for those variables to be escaped so that they resolve to the downstream values. I tried one backslash but obviously that did not work. I then had the idea of editing the file not renaming, and that works, but the more complicated command required eval. I understand the concern about that.

So, we need to find a way to escape CI variables in the string passed to the downstream project. Or we have the cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME in the downstream job, which always gets executed, then conditionally do the PHPCS_BEFORE_SCRIPT command. That is fine here, as it is quite a niche requirement. We know that we want to change directory. That actually makes the command simple. I did have that in an earlier test version, so it's easy to recover it.

jonathan1055 → created an issue.

I have found a simpler way for you to fix the phpcs and phpstan problems, and not have to pin your Gitlab Templates version to 1.6.14 but allow you to return to using the default version. The recursion problems in PHPStan were due to this project having a /web/ folder at the top of the project repository. This is a perfectly valid thing to do, and you do not need to change it. The problem is that the pipeline build process also creates a 'web' folder at top level. The name of this folder is arbitrary and we already provide a variable _WEB_ROOT to hold the value, which defaults to 'web'. But if you specify a different name in your .gitlab-ci.yml file then the problems do not occur.

MR18 now demonstrates this - see the pipeline https://git.drupalcode.org/issue/droopler-3503626/-/pipelines/413632
The phpstan job runs and passes green.
The phpcs job now validly checks your project's 'web' folder, and we see

Registering sniffs in the drupal-project standard... DONE (131 sniffs registered)
Creating file list... DONE (24 files in queue)
FILE: web/profiles/droopler/src/Form/RecipesForm.php
-------------------------------------------------------------------------
FOUND 7 ERRORS AND 1 WARNING AFFECTING 8 LINES

These are valid phpcs errors and warnings, they were not being reported previously because the "web" folder was being ignored (regardless of whether a project had their own or not). That problem was fixed in gitlab templates issue 🐛 PHPCS error in contributed module caused by core recipe.README.txt Active which is what exposed the problem you reported.

I have removed the testing changes, so MR18 is now ready for review