Once upon a time, in the ckeditor5 contrib module, commit-code-check.sh was used to do core-quality code checks on Drupal CI. This necessitated a bunch of awkward sed calls and other very hacky dance moves.
Automatic Updates does the same thing. I'm sure it would help Project Browser too. Or anything else that is developed first in contrib, for eventual inclusion in core.
Make it possible for commit-code-check.sh to run against a particular directory.
This will enable any Drupal contrib module to add this to their drupalci.yml:
container_command.commit-checks:
commands:
- ../../../core/scripts/dev/commit-code-check.sh --drupalci
halt-on-fail: true
… and get the same exact checks as core does:
phpstan.neon.dist and a .cspell.json$ sh ../../../core/scripts/dev/commit-code-check.sh
1/3 /Users/wim.leers/core/modules/contrib/automatic_updates/drupalci.yml 175.53ms
2/3 /Users/wim.leers/core/modules/contrib/automatic_updates/next 4.37ms
3/3 /Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php 55.63ms
CSpell: Files checked: 3, Issues found: 0 in 0 files
CSpell: passed
----------------------------------------------------------------------------------------------------
Running PHPStan on *all* files.
[OK] No errors
PHPStan: passed
----------------------------------------------------------------------------------------------------
............................................................ 60 / 332 (18%)
............................................................ 120 / 332 (36%)
.....S...................................................... 180 / 332 (54%)
............................................................ 240 / 332 (72%)
............................................................ 300 / 332 (90%)
................................ 332 / 332 (100%)
Time: 2.81 secs; Memory: 20MB
PHPCS: passed
----------------------------------------------------------------------------------------------------
Checking next
next passed
----------------------------------------------------------------------------------------------------
Checking drupalci.yml
ESLint: drupalci.yml passed
drupalci.yml passed
----------------------------------------------------------------------------------------------------
Checking package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php passed
----------------------------------------------------------------------------------------------------
phpcs.xml that extends core's$ sh ../../../core/scripts/dev/commit-code-check.sh
1/2 /Users/wim.leers/core/modules/contrib/cdn/interdiff.txt 169.54ms
2/2 /Users/wim.leers/core/modules/contrib/cdn/wip.patch 14.94ms X
/Users/wim.leers/core/modules/contrib/cdn/wip.patch:24:51 - Unknown word (Validatable)
/Users/wim.leers/core/modules/contrib/cdn/wip.patch:34:51 - Unknown word (Validatable)
/Users/wim.leers/core/modules/contrib/cdn/wip.patch:49:2 - Unknown word (farfuture)
/Users/wim.leers/core/modules/contrib/cdn/wip.patch:78:42 - Unknown word (farfuture)
CSpell: Files checked: 2, Issues found: 4 in 1 files
CSpell: failed
----------------------------------------------------------------------------------------------------
Running PHPStan on *all* files.
------ ---------------------------------------------------------------------
Line cdn.module
------ ---------------------------------------------------------------------
53 Function file_create_url not found.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
53 Function file_url_transform_relative not found.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
------ ---------------------------------------------------------------------
------ ---------------------------------------------------------------------------------
Line tests/src/Unit/File/FileUrlGeneratorTest.php
------ ---------------------------------------------------------------------------------
523 Anonymous function has an unused use $base_path.
523 Anonymous function has an unused use $current_uri.
523 Anonymous function has an unused use $root.
556 Class Drupal\cdn\CdnSettings constructor invoked with 2 parameters, 1 required.
------ ---------------------------------------------------------------------------------
-- ----------------------------------------------------------------------------------------------------------------------------------
Error
-- ----------------------------------------------------------------------------------------------------------------------------------
Ignored error pattern #^Plugin definitions cannot be altered.# was not matched in reported errors.
Ignored error pattern #^Missing cache backend declaration for performance.# was not matched in reported errors.
Ignored error pattern #cache tag might be unclear and does not contain the cache key in it.# was not matched in reported errors.
Ignored error pattern #^Class .* extends @internal class# was not matched in reported errors.
-- ----------------------------------------------------------------------------------------------------------------------------------
[ERROR] Found 10 errors
PHPStan: failed
----------------------------------------------------------------------------------------------------
.......................... 26 / 26 (100%)
Time: 293ms; Memory: 12MB
PHPCS: passed
----------------------------------------------------------------------------------------------------
Checking composer.lock
composer.lock passed
----------------------------------------------------------------------------------------------------
Checking interdiff.txt
interdiff.txt passed
----------------------------------------------------------------------------------------------------
Checking wip.patch
wip.patch passed
----------------------------------------------------------------------------------------------------
phpcs.xml that extends core's:$ sh ../../../core/scripts/dev/commit-code-check.sh
1/1 /Users/wim.leers/core/modules/contrib/big_pipe_sessionless/big_pipe_sessionless.info.yml 192.23ms X
/Users/wim.leers/core/modules/contrib/big_pipe_sessionless/big_pipe_sessionless.info.yml:1:7 - Unknown word (Sessionless)
CSpell: Files checked: 1, Issues found: 1 in 1 files
CSpell: failed
----------------------------------------------------------------------------------------------------
Running PHPStan on *all* files.
------ ----------------------------------------------------------------------------------------------------------------
Line tests/src/Unit/Render/Placeholder/BigPipeSessionlessStrategyTest.php
------ ----------------------------------------------------------------------------------------------------------------
59 You should use assertCount($expectedCount, $variable) instead of assertSame($expectedCount, count($variable)).
------ ----------------------------------------------------------------------------------------------------------------
-- ----------------------------------------------------------------------------------------------------------------------------------
Error
-- ----------------------------------------------------------------------------------------------------------------------------------
Ignored error pattern #^Unsafe usage of new static# was not matched in reported errors.
Ignored error pattern #^Plugin definitions cannot be altered.# was not matched in reported errors.
Ignored error pattern #^Missing cache backend declaration for performance.# was not matched in reported errors.
Ignored error pattern #cache tag might be unclear and does not contain the cache key in it.# was not matched in reported errors.
Ignored error pattern #^Class .* extends @internal class# was not matched in reported errors.
-- ----------------------------------------------------------------------------------------------------------------------------------
[ERROR] Found 6 errors
PHPStan: failed
----------------------------------------------------------------------------------------------------
.....E.... 10 / 10 (100%)
FILE: /Users/wim.leers/core/modules/contrib/big_pipe_sessionless/src/StackMiddleware/BigPipeSessionlessPageCache.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
28 | ERROR | Description for the @return value is missing (Drupal.Commenting.FunctionComment.MissingReturnComment)
30 | ERROR | Public method name "BigPipeSessionlessPageCache::_storeResponse" is not in lowerCamel format
| | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Time: 83ms; Memory: 10MB
PHPCS: failed
----------------------------------------------------------------------------------------------------
Checking big_pipe_sessionless.info.yml
ESLint: big_pipe_sessionless.info.yml passed
big_pipe_sessionless.info.yml passed
----------------------------------------------------------------------------------------------------
This would allow every contrib module that chooses to do so, to follow the lead of continuously matching core's evolving best practices.
Implement this in a merge request and manually test it. Then, RTBC and merge it, and rejoice! Standard procedure.
TBD; I'm not sure commit-code-check.sh counts as a user interface.
TBD; I'm not sure it counts as an API either.
None.
TBD, but this being an internal development script, I doubt it will need one.
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | 3314100-22.patch | 11.32 KB | wim leers |
| #22 | interdiff.txt | 1.3 KB | wim leers |
| #19 | 3314100-19.patch | 11.2 KB | wim leers |
| #19 | interdiff.txt | 6.13 KB | wim leers |
| #17 | 3314100-17.patch | 5.71 KB | wim leers |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
phenaproximaComment #3
phenaproximaComment #4
wim leersYES PLEASE!! 🤩
I'm 99% certain the answer is 🤓
Comment #5
phenaproximaThis is blocked by #3314151: Fix cspell use: specify globRoot and always pass --root to cspell.
Comment #6
wim leersThis came up again, because of #3342120: Fix PHPStan violations (happened because PHPStan code checks stopped running…). Let's make this happen 🙏
Comment #7
wim leersI've fixed #3314151: Fix cspell use: specify globRoot and always pass --root to cspell. Please review, so we can move forward here :)
Comment #8
wim leersComment #12
wim leersRather than recreating this MR to target
11.x, I'm just uploading it as a patch. (Also, the conflict that #3314151: Fix cspell use: specify globRoot and always pass --root to cspell caused was not resolved correctly in the existing MR. 😅 But it's tricky!)Also because creating a new branch endlessly tells me 🤪😬😬
This patch correctly resolves the conflict: #3314151: Fix cspell use: specify globRoot and always pass --root to cspell already did everything we needed! 😊
Comment #13
wim leersWe can do better than this I think.
This means that you would have to first
cdinto the contrib module and then specify the Drupal root manually:… but the script is already in Drupal core. So why don't we use that to determine the Drupal root? 🤓
Comment #14
wim leers#13 works fine:
Note that
cSpellautomatically picked up the.cspell.jsonin that module (i.e. the current working directory)!Contrast that with:
That module does not have a
.cspell.json👍Next up:
phpstan.Comment #15
wim leersFirst let's stop running all tests, we only want the commit checks. Let's save some DrupalCI cycles 😊
Comment #16
wim leersApparently we need some test results for it to succeed.
Comment #17
wim leersNow also works for
phpstan:👆 This means it works, because if it were using core's
phpstan.neon.dist, it would fail: there's additionalignoreErrorsentries! 🥳Comment #19
wim leersAnd now everything else:
phpcsfor FILE in $FILES; do, running:stat permissions checksphpcs on YAMLeslintPCSS → CSS compiling👍
Specifically skipped for non-core projects, because they're highly specific to Drupal core:
yarn run -s lint:core-js-passingyarn run -s lint:cssyarn run -s check:ckeditor5yarn run build:css --checkAnd under
for FILE in $FILES; do, skipped these:# Don't commit changes to vendor.# Don't commit changes to core/node_modules.phpcs on PHP files→ because we've already scanned everything above, so no more need to scan one-by-one!Resulting output:
👆 this means that
modules/contrib/automatic_updates/drupalci.ymlcould use:… and that's all it would take for a contrib module to adopt this!
Comment #20
wim leersWhile the original title was accurate, I think this title better reflects the ecosystem impact this is aiming at! 😊
Comment #21
wim leersAdding a second and third example to the issue summary.
Comment #22
wim leersI got something wrong in the PHPStan one.
Comment #23
phenaproximaTests not passing...?