Yes. I also was thinking about opening an issue in Webform, as this would probably not be compatible with 11.3 when it is released. I know that some projects do not like "chasing core" to remove deprecation, but this error does not seem like a deprecation. I would guess it would at least cause webforms phpunit tests to fail when 11.3 becomes 'current' but I will check that before raising an issue.
With webform removed the phpunit 'next minor' runs OK https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/65...
We get the expected 7 deprecations for search_api, which are beig fixed in MR35 on 📌 Deprecations in 3rd-party modules in Next Minor Drupal 11.3-dev Active
The immediate solution is to drop webform → from the dependency list. Then we can get a green passing pipeline and continue working as before.
We can add it back later, when the problem is fixed in webform and it works with 11.3-dev.
But, if you meant that it would get it from the
PROJECT_TYPE, it seems this value comes from the*.info.yml, which Recipes don't have.
Yes I did mean the calculated value, sorry, not the actual default value. I was not aware that recipes do not have a .info.yml (actually, I've not worked on any recipe projects and don't know much about them. Maybe it was an assumption of mine that all D8+ projects should have a *.info.yml)
We have an open issue and MR for documentation updates 📌 Documentation pages Active or you can make a new MR here.
Oh, that is excellent. I didn't know that you could have executable php in an include file
The second suggestion was the solution I thought of when creating this issue, and is also my preferred option. But I recalled the slack conversation
One option could be to just write/append the new patttern into the existing file (which would require determining where the ignoreErrors: section is, or adding it if none.
However, the phpstan.neon file can also inlcude secondary files, like:
includes:
- phpstan-baseline.neon
- phpstan-baseline-extras.neon
So a second approach could be to add an inlcude: line and have the extra messages in an asset file in the gitlab_templates project. This way could be clearer to understand and maintain.
ha! Found it! A quick look through the 11.x commit log and I found this commit on 12 September from 🐛 Deprecation warning in DefaultPluginManager::__construct() is too unspecific Active
I suspect that we no longer need the "core" part of the pattern now, so I will remove that and check. It did make me wonder at the time when I was doing the first MR, why we were seeing a deprecation that appeared to be a core problem and that there was no indication that it came from a 3rd-party contrib module. This now all makes sense!
Thanks for replying @karavkars.
As there has been no response from the creator of the issue for a month (since my last comment #8 on 16 Aug) and no response for 3 months (since my request for info in #7) I am closing this issue and the merge request.
Downstream d10-plus with _COMPOSER_YARN_INSTALL=1
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
All looks OK
Downstream d10-theme with _COMPOSER_YARN_INSTALL=0
The following surprised me https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/65...
Displaying the version reverted Yarn to 1.22.22. I obviously don't understand the architecture and what is being done here :-)
Full pipeline https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
OK I will work on that. I don't think its very helpful (indeed could even be more confusing) if the versions are shown in the Composer log when _COMPOSER_YARN_INSTALL=0 because showing that info implies that something has been executed, when in fact nothing has been done. This was my addition, so I will remove it. It may be helpful to instead show a message something like "_COMPOSER_YARN_INSTALL=0 therefore node_modules corepack and yarn install will be done automatically in the jobs that need it. See https://project.pages.drupalcode.org/gitlab_templates/jobs/composer/#whe... for more details"
The version info should always be shown in the later jobs that use it, regardless of the value of _COMPOSER_YARN_INSTALL. The only question is whether corepack enable should also be run, even if not installing, as that has an impact of the version of Yarn that is shown. But would I be right in thinking that if there is no yarn install then the version of Yarn is irrelevent?
All jobs are green, specifically the PHPUnit at 'next minor'
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Ready for review
Thanks for the hint. Just tried that, and we get the same deprecations with PHPUnit 11.5.36 (which I'm not surprised at, but worth eliminating from the equation)
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...
I might just have to accept that I don't know why it was all clear on commit on 6th Sept, but by 16th Sept we had new deprecations. The pattern to ignore these was already in the file for the Core deprecations of this format, so it was "active" back on 6th, but just not being reported for webform and search_api.
Thanks @duaelfr
Yes this issue was created in the early days of Gitlab Templates, probably before the pipeline variants were introduced. The variable requested here did actually get added.
The variables list are precisely the ones that the "OPT_IN" variants use. See the project documentaion page gitlab_templates/info/variants/ for more details.
The 13 new deprecations are all in search_api (7) and webform (6) are all of the form
"Not supporting attribute discovery in * is deprecated in drupal:11.2.0"
- The search_api version has not changed, still 1.38.0
- Neither has the webform version 6.3.0-beta4, these are both the same as in 6th Sept.
- The core/.deprecation-ignore.txt did have a recent change but it was not related to this pattern
- I have compared the logs, and the only salient differences are that the Gitlab Runner has been updated 18.3.0 -> 18.3.1 and PHPUnit 11.5.36 -> 11.5.39. Could this have caused the difference in behavior?
I don't think we can easily run phpunit with the earlier version, can we?
The final pipeline in MR33 ended green for 'phpunit next minor' which implied to me that all the deprecations were covered in the new .deprecations-ignore.txt file added here.
However, on running 'phpunit next minor' now, there are more deprecations not ignored and the job ends with the amber warning.
See this scheduled d10-plus branch test from last night
I don't know if these deprecations have been added since the last run on 6th September (probably not), or whether somehow they were there all along but not being reported in the job (more likely, but I don't know why). Either way, patterns for these deprecations can be added easily into the .deprecations-ignore.txt file.
I can see why this difference is happening. The Merge Request has been rebased so the 3523139-test-only-changes branch now includes the two commits made to the project's d10-plus branch (Sep 6 and Aug 21) - see gitlab_templates_downstream-3523139/-/commits/3523139-test-only-changes
But the d10-plus branch in the issue fork does not have these two commits, see gitlab_templates_downstream-3523139/-/commits/d10-plus. So the test-only changes job determines the common ancestor SHA to be 520125b and we get those extra changes in the list.
I ran the job again via UI adding our new variable _TEST_ONLY_DEBUG=1 and get more info in the log
This is aall fine, I don't think there is any other question to solve here. I could update the issue branch, but have not done so, because I want the links above to demonstrate the difference. I am going to remove the temporary code then this will be ready for merging.
Using GTD d10-theme branch with _COMPOSER_YARN_INSTALL=0, therefore running Drupal 11 for the validation jobs -
The composer job shows yarn version 1.22.22
The eslint job and cspell job show 1.22.22 at the start, but after corepack enable</ code> the yarn version is immediately 4.9.1. So it is nothing to do with the <code>yarn add @gitlab-formatters in stylelint, which is good in a way, as at least we have consistency over all the validation jobs, even if we don't yet understand why the version is reported differently. I have to admit that I know very little about node_modules and corepack and yarn, so all I'm doing here is providing the info and debug.
Our aim is to have better and clearer info in the logs, so maybe @tom konda who created the request has an idea about what could be going on?
To correct the unwanted installation I have removed the call to *get-node-modules from the Composer job (as it can't be executed there if you don't want the check on job name). So I added the version display lines directly.
Regarding the changing yarn version, I have added some extra debug to investigate:
In this d10-plus composer job (_COMPOSER_YARN_INSTALL=1) it seems to be just corepack enable which causes the newer version as we can see it jumps from 1.22.22 to 4.9.1 right there.
In the linting jobs that do not run corepack enable (because it is all done in Composer job) the version is reported as 1.22.22 and remains that way. This could be confusing for the original purpose of this issue. Is the version really 1.22.22? See this d10-plus eslint job
In d9-basic (_COMPOSER_YARN_INSTALL=0) the composer job just shows 1.22.22 because no enable or install is done.
But confusingly, in the d9-basic eslint job, where the enable and install is done we still see version 1.22.22 at the end. Even in the d9-basic stylelint job it is 1.22.22. This could be because this pipeline is running the linting jobs at Drupal 10 not 11. I will run a test on d10-plus with _COMPOSER_YARN_INSTALL=0 to see what we get.
The default value for DRUPAL_PROJECTS_PATH should already cater for 'recipe' projects - see
https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...
I've restored back to before. It means that we have corepack enable && yarn install in two places, which is what I wanted to avoid, but it's OK, no big deal.
d10-plus pipeline with default
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
I have now commited _COMPOSER_YARN_INSTALL: 0 to the d9-basic branch, and removed the temporoary line
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
This is actually now doing the wrong thing, because I used your suggestion of still calling the reference in the Composer job. So that means that the directory does not exist and it gets created even when not wanted. So I will have to change that again.
Also it is interesting to see that the inital version didslayed for yarn is 1.22.22 as shown in this cspell job, but after yarn install it shows as 4.9.1 which can be seen in this stylelint job.
I have pushed a modification to allow the get-node-modules reference to be called also in the Composer jobs. This means that the versions are displayed in the composer log too, and if we ever had to change how the corepack enable and yarn install are done, it would be in one place not two.
GTD d10-plus uses the default _COMPOSER_YARN_INSTALL: 1
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
GTD d9-basic has _COMPOSER_YARN_INSTALL: 0 and we see the different log output
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Reeady for review and feedback. I think it would be good to have the _COMPOSER_YARN_INSTALL: 0 commited into one of the GTD branches permanently, so we cover that scenario always, therefore the chnage in .gitlab-ci.yml here is only temporary.
OK. I have some ideas about consolidating the two places where we do yarn install, and this seems like a good time to do it. It's the weekend, so this can wait till Monday :-) Just letting you know not to commit this MR right now, if that's OK?
Thanks for the quick response. I confirm that this is now fixed.
Good addition.
What happens if _COMPOSER_YARN_INSTALL is 0 so that it's not installed already in Composer, and gets installed via the modified .get-node-modules snippet. Do the new outputs still work? Just wondering if we have a downstream project with this setting. If not, it would be useful to do so. Or we can push a temporary change to set _COMPOSER_YARN_INSTALL: 0 in the downstream pipeline definition.
Yes, a name without the issue number might be better. It could be a separate branch in the project, or it could reside in that issue fork as now. A new separate branch in the project has an overhead of maintainance, we already have seven branches. It all depends on if we want a new separate downstream pipeline (as linked in #33 above) or whether we want to incorporate the 'test-only changes' job into the existing d10-plus pipeline. I think with using the _TEST_ONLY_TARGET_BRANCH variable we may be able to run the job in the existing downstream pipeline (not an issue fork) and compare to another branch. But I'm not certain how that would detect the changes. Have to test it.
I think my preference would be not to have a new (8th) branch in the project just for this, but to have the changes in an issue fork which can be easily updated via the 'update fork' UI button. Actually, I was imagining that we'd just keep the existing GTD MR29 open and use that branch for the test. Then it would be easy to rebase the MR if/when d10-plus received updates.
Checking without the core patch that reverted a test discovery change:
https://git.drupalcode.org/project/scheduler/-/jobs/6514645 5:55 (run-tests.sh), 6:42 (whole job)
https://git.drupalcode.org/project/scheduler/-/jobs/6515061 4:49 (run-tests.sh), 5:34 (whole job)
https://git.drupalcode.org/project/scheduler/-/jobs/6515197 5:53 (run-tests.sh), 6:58 (whole job)
Here are the changes since 8.x-1.5
https://git.drupalcode.org/project/scheduler/-/compare/8.x-1.5...8.x-1.x...
I confirm this is the case on a full-screen laptop (not tried on small-screen phone).
The simple solution would just to not have that button/link when editing the issue summary, as they are likely to be separate tasks. Or have it placed at the end of the page along with the other links.
Milestone moment! We have our first "test-only changes" job being run in a downstream pipeline, triggered from this Gitlab Templates MR :-)
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/p...
The $CI_PIPELINE_SOURCE is neither 'web' nor 'merge_request_event', it is 'pipeline'.
For this I added a new GTD downstream pipeline which use the 3523139-test-only-changes branch in the issue fork. This branch can remain in-situ, and becomes the permanent place where the 'test-only changes' job is run downstream. The content of that branch is not finalised yet - see
✨
Add coverage for test-only changes job
Active
, but at least we now have proved the concept of running 'test-only changes' in a downstream pipeline.
No changes to the Scheduler tests, but things run faster now anyway:
12 Sep
https://git.drupalcode.org/project/scheduler/-/jobs/6513772 4mins 48
https://git.drupalcode.org/project/scheduler/-/jobs/6513931 5 mins 38
https://git.drupalcode.org/project/scheduler/-/jobs/6514358 5 mins 33
This may be due to using a lower value for concurrency threads, see 📌 Lower default concurrency Active which was mered in July 2025.
Made title more specific and added related issue for cross-reference.
Updated title to show the default is still 32, as there was discussion about reducing it in this issue.
For BC, the biggest dependent project is Scheduler Content Moderation Integration → so we'd need to test thoroughly with that. I want to run a test on SCMI anyway using this MR254 before merging, just to make sure.
The test coverage we have in Scheduler is pretty thorough (I have made sure of that over the years) so I'm fairly confident that your changes here are good to go in. Yes a new release is worth it, for these improvements.
Actually, I've changed my view on #12 and maybe it would be better to do the simplification and removal of redundant code in a separate MR in a follow-up issue, and keep MR254 just for the caching improvements. Then it will be easier to see what is being changed/removed later, after this one is committed.
The single fix for this deprecation reduces the number of reported deprecations in the Scheduler group. Previously the 23 tests ranged from 16 to 12 deprecations. The DefaultTime test had 16, but now has only 15. The message does not appear in any test.
All tests pass at D11, D10 and D9. The D8 single test failure is a known (and unrelated) problem.
With these four constraint annotation and void fixes, the scheduler group test log is reduced from 11,280 to 8,659 lines. There were previously tests with 16 deprecations, but we now get a maximum of 10.
All tests run OK at:
- Drupal 11.2.3, PHP version : 8.3.25
- Drupal 10.5.2 , PHP version : 8.1.33
- Drupal 9.5.11, PHP version : 8.1.33
However we get PHP syntax parsing problems at in Drupal 8.9.20, PHP 7.4.28. So before this is committed we need to check the minimum PHP version that this 2.x branch is compatible with, and increase it. This will also mean we should probably stop testing at Drupal 8.
📌 Add #[Group] attributes for Core 11.2 and PHPUnit 11 Active is now done, so work can re-start on this issue.
This could be done in separate issues.
📌
Autoloading hooks in the file tokens.inc is deprecated D11.2
Active
is already open for the hooks autoloading
Thanks for the updates and its great that the tests are passing. Nice when it's something simple to fix.
- the whole $isBundle stuff can go.
- getTypes() is unused now
- The protected properties in \Drupal\scheduler\SchedulerPluginBase::entityTypeFormIds and ::entityFormIds() are likely pointless
If you wanted to make these chages, please go ahead now. Or we could commit MR254 then keep this issue open and make those changes in a second MR here too. You know what you are doing, so it make sense for you to do it whilst you have it in your head.
I pushed a changed to filter for just the failing tests, to focus on those. When passing we can go back to the full set.
https://git.drupalcode.org/project/scheduler/-/jobs/6484435
However this means running _PHPUNIT_CONCURRENT: 0 i.e. using the plain phpunti binary, without concurrency, which is slower. I think there is a better way to filter on just a few test classes and still have concurrency using run-tests.sh.
I have merged the main work MR242. This requied the new PHPStan (previous Major) job to be skipped as the new attributes and classes have not been backported to Drupal 10. However, there should not be any problem with running on a Drupal 10 site, as the attribute classes will not be examined or used in any way.
See this slack discussion on the subject. Specifically
using a non-existing class is still valid, you just can't call it or anything in it. That comes to our benefit when we have e.g. a plugin that will only be used when some other module is installed. And if not, that plugin with strictly speaking invalid use statements will just sit around silently, not doing anything.
Hi macdev_drupal,
That's a good question. Yes I did write that on the project page. The last 8.x release was
8.x-1.5 →
in January 2023. The 8.x branch has been running Gitlab CI pipelines at Drupal 11 sucessfully since October 2024. Maybe it is time to release 8.x-1.6? Currently running on 11.2 with no problems. Have you used the 8.x dev release in a Drupal 11 site?
The issue fork needed updating, which I've done. You have all the commits from the previous branch in yours, but the overall changes are just your new ones, so I guess this is OK.
jonathan1055 → changed the visibility of the branch 3473000-doc-update-8 to hidden.
The pattern manipulation works fine now. Test via UI with \.module specified in the test pattern, so .module is not reverted (and neither is .deprecation-ignore.txt) and the test passes green as intended.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/j...
One thing I noticed is that, when running via the web UI, the derived SHA to compare back to has two more commits than when run just in the MR. Here is the job direct from MR
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/64...
d10-plus SHA 5bb804582959db2377cbd37980211a23e4d2f61e
5 files changes
But the job from web UI
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/j...
d10-plus SHA 520125b0b88cbbbefd581174aaf254a0a98639c2
6 files changed, new script
Attached is an image of the git history, highighting these two commits. But it might be working fine and does not indicate a problem in the recent test-only work we did to allow it to run from the UI. You can see the info on the history. Maybe the issue fork is not up-to-date and that's why we get the extra two commits? Also I had to recreate this branch, and it may not be typical.
So and apart from the above question, ansd restoring the skipped jobs, this is ready for review. The purpose is to make sure the test-only jobs works correctly (hence the deprecation stuff) and also to add a simple function that can be modified in the real branch where we will do the downstream testing.
Thanks, that's really good to know, and thanks for taking the time to write the demo script.
The version that is in the MR at present is
# Add .deprecation-ignore.txt to the file pattern to be preserved. Include | if there is already a value for the pattern.
_TEST_ONLY_FILE_PATTERN+=$([[ $_TEST_ONLY_FILE_PATTERN == "" ]] && echo "" || echo "|")"\.deprecation\-ignore\.txt"
It's longer, and still not that readable. So I might use your new shorter one. I like to use new things that I've learned, and the comment should explain it adequately.
It was not that line that was causing the pipe syntax error. The new comment I added at the same time started with a - which I had not noticed. That is what the problem was, and it all works correctly now. I've rebased to remove all the intermediate commits.
With no pre-existing value in the pattern
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/64...
Using UI and setting the pattern to '.module'
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/j...
or I just repeat the "extra"
A+=$([[ $A == "" ]] && echo "extra" || echo "|extra")
The conditional is working fine, it is the final concatenation that isn't. It just bugs me that I can't work out the correct way. The problem is that bash in the pipeline script does not behave exactly like the bash in my local terminal, so I'm taking guesses and pushing to see what happens :-)
Still working on this. MR32 will be committed first. MR29 might remain in draft, for use in the downstream testing, not sure yet.
Current problem should be really simple but I've not found a solution. It works in local bash terminal but not in the pipeline. In words, I want to add the string \.deprecation\-ignore\.txt to the variable $_TEST_ONLY_FILE_PATTERN but if there is already a value in that variable, it needs a pipe | before the additional text. Simple. Or to make it short and readable, use $A as the variable, and extra as the text.
A+=$([[ $A == "" ]] && echo "" || echo "|")+"extra"
A+=$([[ $A == "" ]] && echo "" || echo "|")"extra"
A+=$([[ $A == "" ]] && echo "" || echo "|")extra
The above ways to add 'extra' all fail the pipe syntax, even though OK locally. I have also tried it inside a piped section. The pipeline fails to be built - eg https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
No problem. Actually I was not expecting you to review it, so I'm not sure why I set to NR. I knew you have a lot else on right now.
d9-basic does not need the .deprecations-ignore.txt file because that branch is designed to be tested at Drupal 9 and 10 only. However, there were some other minor changes that I have replicated from d10-plus, to keep things aligned where appropriate.
The full pipeline is green, so I am going to merge this.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
I will investigate if we need to cherry-pick to d9-basic. I don't think we do, but would be good to have the module enabled in the tests anyway, to keeo as much aligned as possible.
After the addition of the projects own .deprecation-ignore.txt the job shows a reduction down to 15 deprecations
First commit demonstrates the problem. Only group gtd1 in 'next minor' fails, because this is the test which enables the module. No failure at 'current' because that uses the default setting.
jonathan1055 → created an issue.
I have worked out where the difference is coming from. The phpunit 'next minor' job is defined in the .gitlab-ci.yml with _PHPUNIT_CONCURRENT=1 so that we use run-tests.sh and can detect deprecations via SYMFONY_DEPRECATIONS_HELPER: "ignoreFile=$CI_PROJECT_DIR/$_WEB_ROOT/core/.deprecation-ignore.txt" as intended, and this is what we see in the full MR pipeline phpunit next minor job
But I think the reason that the phpunit next minor job initiated via the UI passed green is that the .gitlab-ci.yml file does define any _PHPUNIT_CONCURRENT at the top level, so this value appears in the form, and it has its default value of 0. This value is applied to all the phpunit variants, and even overrides the customised value of 1 in the 'next minor' job. So 'next minor' therefore uses the phpunit binary, and does not display the deprecations. Is that because it does not use the SYMFONY_DEPRECATIONS_HELPER variable? Our help page implies that it should work in either concurrency setting, but we need to verify that.
Actually this needs a bit more checking before commit. The clean green run was a second test I did, via the web UI, to use _TEST_ONLY_FILE_PATTERN: "\.module" so that the .module was not reverted in the test-only job. That worked ok, and all the phpunit jobs (current and next minor) are also green.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/p...
But running the entire pipeline via a push to the MR branch - we get a different result for 'phpunit next minor - group 1'. It fails amber with a deprecation, this is probably because that test now installs the gitlab_templates_
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
But I do not understand why when running from the UI that phpunit job ended green. I'm not sure we want to commit to the real d10-plus branch when 'phpunit next minor' ends amber, as that will just cause us extra work and investigation when we see that in the GTD downstream pipelines.
In preparation for the MR downstream test, here a a few changes
https://git.drupalcode.org/project/gitlab_templates_downstream/-/merge_r...
All phpunit tests (current and next minor) and the test-only job are green, using current latest release of templates, which is 1.11.2
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/p...
So I'm going to commit this, then re-work the MR29 which is the branch we will use in the downstream test.
Thanks mondrake, yes you are right. I had forgotten that solution, and have updated the issue summary. This can be closed now.
jonathan1055 → changed the visibility of the branch 3539542-test-only-patterns to hidden.
Thanks for doing the version changen quickly. Helps not ot have MRs failing.
Nice to see the new contribution records live, well done on that massive milestone.
It would have been useful, but I guess if it didn't happen back then, it won't be done now. There was also not a clear agreement.
We do still have this type of problem in Contrib at 11.2 - see the list of 3rd-party modules in the summary of
📌
Autoloading hooks in the file tokens.inc is deprecated D11.2
Active
which clouds the log with noise and affects the test results.
But thanks for following up, though. If no one else picks this up it can be closed.
I used the loop with read and <<< "$CHANGES_TO_REVERT" which worked locally in testing and passes shellcheck. Thanks to @codebymikey again.
In theory I should really fix the other array to loop bits, as they also do not work properly locally. They do work OK in the gitlab pipeline job, so I guess that is why we've not seen a problem so far.
Testing via UI still works as expected, in that the correct tests are run. But even though Exiting with EXIT_CODE= when all tests passed, we still get ERROR: Job failed: command terminated with exit code 1 and the job ends amber. Not sure why that is, as it used to end green if no tests failed. More investigation needed.
Now also I can start work on the downstream projects, to see if we can test there too (ie not MR and not UI)
Ah, thank you @codebymikey, those ideas are great. Thanks for the tip, I will use that site too.
I have just discovered, though, that locally I get different behavior if run the script by calling it directly (all works OK, newlines in the list give separate loop values)
CHANGES_TO_REVERT=.prettierignore
scheduler.module
scheduler.services.yml
==start of loop
File - .prettierignore
File - scheduler.module
File - scheduler.services.yml
==end of loop
Whereas if I exuecute using source /the/script.sh it fails as above, with
CHANGES_TO_REVERT=.prettierignore
scheduler.module
scheduler.services.yml
==start of loop
File - .prettierignore
scheduler.module
scheduler.services.yml
==end of loop
The existing (as in main, before any changes in this MR) without the extra wrapping parenthesis, locally gives all the changed files in one "string" but includes line breaks. The printed output is:
CHANGES_TO_REVERT=.prettierignore
scheduler.module
scheduler.services.yml
Then the for loop acts on the entire value in one, so it tries
executing: git checkout 89456f774b3d226c0234d055c6e281fbd1fd1ff4 -- .prettierignore
scheduler.module
scheduler.services.yml
which gives the error
error: pathspec '.prettierignore
scheduler.module
scheduler.services.yml' did not match any file(s) known to gitadding some wrapping double quotes to the part inside the parentheses might fix
I saw that and tried, but no better.
I did try to alter the for file in $CHANGES_TO_REVERT; do but without success. Maybe I should try that angle again, and not alter the way the string in generated.
Now that 📌 Pass a "d7" parameter into scripts/test-only.sh and discard scripts/test-only-d7.sh Active is done and we no longer have to make near-duplicate changes in a d7 script, I have merged that in.
Testing the script locally was not working for me (different flavor of bash / zsh) - in particular the array expansion from a string of changed files created by CHANGES_TO_REVERT=$(git diff ${BASELINE} --diff-filter=DM --name-only | grep -Ev $PATTERN). The for loop was only ever getting the first item, even if I referred to it using the longhand "${CHANGES_TO_REVERT[@]}". A solution that worked was to add a second set of ( ) around the definition.
This also works in the pipeline job and the correct files are reverted.
The problem is this now fails ShellCheck
In scripts/test-only.sh line 71:
CHANGES_TO_REVERT=($(git diff ${BASELINE} --diff-filter=DM --name-only | grep -Ev $PATTERN)) || true
^-- SC2207 (warning): Prefer mapfile or read -a to split command output (or quote to avoid splitting).
For more information:
https://www.shellcheck.net/wiki/SC2207 -- Prefer mapfile or read -a to split command output
See the Check Code job log and and https://www.shellcheck.net/wiki/SC2207
None of the suggested resolutions work for me locally, so I don't think I'm on the right track. Maybe the original default array expansion was OK left as-is, as it works in the Gitlab job, just not in my local terminal session where I am doing the development. There should be a way round this, or maybe I just change the code to test, then revert before pushing. But that has many dangers.
jonathan1055 → changed the visibility of the branch 2.x to active.
When this is committed I will merge it into ✨ Allow users to specify custom files that Test-only changes Active then continue on that.
Thanks, yes they are good and I have applied those suggestions.
Thanks for the feedback, I agree with your suggestions and have made them.
Re-tested for D10 on Scheduler MR253
Using _PHPUNIT_CONCURRENT:1 to run run-tests.sh
https://git.drupalcode.org/project/scheduler/-/jobs/6394117#L487
D10 using _PHPUNIT_CONCURRENT:0 to run phpunit binary
https://git.drupalcode.org/project/scheduler/-/jobs/6393579#L487
Tested D7 via Scheduler MR252
This also checks deleting a newly added file
https://git.drupalcode.org/project/scheduler/-/jobs/6394208#L455
The temporary code was not actually needed here, as all testing is in other real MRs not downstream (yet), so I have removed that, and this is now ready for review again.
Next change is to include docs in the files checked by phpcs. It fails, mainly due to the long lines which is to be expected
https://git.drupalcode.org/issue/gitlab_templates-3543301/-/jobs/6386981
Adding summary and source reports, via the phpcs.xml file, and excluding the linelength sniff from .md we get just a few failures, which can be fixed
https://git.drupalcode.org/issue/gitlab_templates-3543301/-/jobs/6387157
The first change is to use the short class name for the standards. We did this for the default contrib phpcs.xml.dist, but forgot to do it for out internal phpcs.xml. Job runs OK
https://git.drupalcode.org/issue/gitlab_templates-3543301/-/pipelines/58...
The second change is to simplify the phpcs commands by setting the flags and options in the phpcs.xml file instead of on the command line. All OK, and it also works OK locally
https://git.drupalcode.org/issue/gitlab_templates-3543301/-/pipelines/58...
I've looked into this, and found some improvements we can make to the Check Code phpcs to highlight and fix things in .md files . However, I have found that the trailing spaces sniff Squiz.WhiteSpace.SuperfluousWhitespace.EndLine will not actually detect trailing spaces in .md files because it only works inside <?php tags.
I will push the other changes here anyway, then maybe make an enhancement to our unformatted-links.php script to detect trailing spaces. That should be easy to do, and it will be a while before any of the issues listed in the summary are resolved and we get an "official" job to do it.
"Thanks for fixing the link checker code."
Well, it was my invention and so I felt I needed to fix it :-)
This MR dealt with a lot of the doc updates that I had been collecting, but I still have a bunch of things for the phpunit job page (which were too complex to quickly add in here this time). Setting back to 'active' to prevent closure in two weeks.
Also we have some internal standards and conventions for our jobs and documentation. It might be nice to have them recorded in a page somewhere. Maybe that's a new separate issue, and it could wait until we can test it with 📌 Allow testing documentation pages via MRs Active
grimreaper → credited jonathan1055 → .
Tested OK in downstream d10-plus and d9-basic which do not have any prettier config files.
In the 3503337-mr-testing-d10-plus branch, the log shows that it uses the local stylelint and prettier config files.
MR402 is ready for review
As per the MR comments I have reverted the change to the job rules: and will add that on
📌
Allow testing documentation pages via MRs
Active
instead.
The "check code" job fails with
Loading composer repositories with package information
In CurlDownloader.php line 394:
curl error 28 while downloading https://repo.packagist.org/packages.json:
Failed to connect to repo.packagist.org port 443 after 10005 ms: Timeout was reached
I re-ran the job and failed in the same way, but I think this is an unrelated error, nothing to do with the changes in the MR.
The doc pages were rebuilt in the earlier job and can be checked here, so this is ready for review.
Do you think we should make an announcement on Slack that styleleint jobs are now using the core prettier config when they were not doing so before. Quite a few project pipelines are retruning new warnings in Stylelint where previously nothing was being checked, or it was using less rigorous set of rules.
Also, in writing the documentation update for Stylelint, I had the thought that there could be alternative file names for the prettier config file, just like there is for the stylelint file, and there are lots of formats. So I will make a quick MR here to cater for them, just for completeness.
And due to your recent PR on DDev-Drupal the local behavior is now good too
https://github.com/ddev/ddev-drupal-contrib/pull/148
Thanks for doing that.
I found a bug in our scripts/unformatted-links.php which only checked the first url in each line. I had errors in the updated .md but the check code job did not detect them. I've fixed the script now, pushed the change, and the job fails as required.
The links are now fixed and the updated stylelint page can be viewed in the repo here. I have made a few more changes that were on my list, and this is now ready for review and feedback.
jonathan1055 → changed the visibility of the branch 3356800-pipeline-rules-in-mr-or-on-commit to hidden.
I've created MR401 here
Downstream jobs are fine.
Tested on the same GTD testing branch as above and now we don't get the error and we do get the local config being used.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
The job fails, which is expected because the local config has different rules.
Ready for review. I was going to add the missing help here, but actually I will do that on 📌 Documentation pages Active so thet we get those other help fixes in too.
Here's an example of the error
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...
But in this situation the outcome is OK, because the blank value of STYLELINT_CONFIG is actually what is wanted here. So maybe we don't need to change anything, as it does give the correct result.
I was trying out something else in a local terminal and got an error saying test: too many arguments which was caused by the new line
STYLELINT_CONFIG=$(test -e .stylelintrc* -o -e stylelint.config.* && echo "" || echo "--config $CI_PROJECT_DIR/$_WEB_ROOT/core/.stylelintrc.json")
After further investigation I realised that test is only expecting one file, and if the wildcard causes two or more files to be returned we get this error. So I think it might be better to replace the above with something like
ls -a | grep -E -q '\.stylelintrc*|stylelint\.config\.*' && STYLELINT_CONFIG="" || STYLELINT_CONFIG="--config $CI_PROJECT_DIR/$_WEB_ROOT/core/.stylelintrc.json"
Locally this copes with multiple matching files. I know this is an edge case, but it would be nice to avoid the error, given that we have just introduced this new part of the job.
Hi frob,
Thanks for raising this issue. Whilst the existing screenshots do still show what the module does, you are right that they could do with refreshing.
If you could provide the screenshot filess here, that would help the maintainers.
Absolutely no problem and no apology needed. I could have also set this back to NW to save you falling into your own trap ;-)
I will do the work on MR328, as that needs to be merged anyway, no need for another MR here.
jonathan1055 → created an issue.
The solution to run cspell in the project folder does not work due the symlinks. This was tried in
🐛
CSpell checks files that are created by a GitLab CI job
Active
and so the "recipes" folder has been added to the $non_project_directories array in prepare-cspell.php - see this commit
However, the other items I mention in the comments above should still be done. We can re-purpose this issue for those.
I knew there was an existing issue about /recipes 📌 CSPELL is checking recipes/readme.txt - change to run in project folder Active . I have no idea why my search in #34 failed to find this issue. It also has some other things to fix in cspell so those could still be fixed over there.
Ah, you said "RTBC pending the documentation changes" and I had not made those changes yet. I thought that was a bit of a gamble to set RTBC early.
No worries, to save a further MR here I will make the changes on MR328 on 📌 Documentation pages Active . There are some other things already in there, so will be good to get that merged soon.
Also could @jibran or @fjgarlin close MR390 as it is not going to be merged now.
Thanks for merging.
Could you also close MR369 which was the first attempt on this issue.