Problem/Motivation

As title, https://www.drupal.org/project/coder/releases/8.x-3.8 released on 24 Mar 2020

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 interdiff-7-11.txt1.2 KBjungle
#11 3121885-8.9.x-11.patch4.96 KBjungle
#7 3121885-7-9.0.x.patch2.19 KBjungle
#7 3121885-7-8.9.x.patch3.76 KBjungle
#7 3121885-7-8.8.x.patch3.74 KBjungle
#5 3121885-5-9.0.x.patch5.95 KBjungle
#5 3121885-5-8.9.x.patch10.99 KBjungle
#5 3121885-5-8.8.x.patch11 KBjungle
#2 3121885-2-9.0.x.patch6.22 KBjungle
#2 3121885-2-8.9.x.patch11.26 KBjungle
#2 3121885-2-8.8.x.patch11.27 KBjungle

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
Status: Active » Needs review
StatusFileSize
new 3121885-2-8.8.x.patch11.27 KB
new 3121885-2-8.9.x.patch11.26 KB
new 3121885-2-9.0.x.patch6.22 KB

Updated by reinstalling, correct me if this is a wrong way, thanks!

$ composer remove drupal/coder -vvv --dev && composer require drupal/coder -vvv --dev

cilefen’s picture

cilefen commented
Status: Needs review » Needs work

Actually the way it should be done is:

composer require --dev drupal/coder:^8.3.8 --update-with-dependencies

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: Unassigned » jungle

Thank you @cilefen for reviewing and correction!

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3121885-5-8.8.x.patch11 KB
new 3121885-5-8.9.x.patch10.99 KB
new 3121885-5-9.0.x.patch5.95 KB

Trigger the tests against 8.8.x and 9.0.x later if 8.9.x patch passes

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
  1. +++ b/composer.lock
@@ -881,10 +881,16 @@
+            "source": {
+                "type": "git",
+                "url": "https://github.com/drupal/core-project-message.git",
+                "reference": "ec887a5cd54543c91085bbb59d9d20ed1c21d922"
+            },
             "dist": {
-                "type": "path",
-                "url": "composer/Plugin/ProjectMessage",
-                "reference": "3b795f469441eb27854798f70cb38e717d80bbfc"
+                "type": "zip",
+                "url": "https://api.github.com/repos/drupal/core-project-message/zipball/ec887a5cd54543c91085bbb59d9d20ed1c21d922",
+                "reference": "ec887a5cd54543c91085bbb59d9d20ed1c21d922",
+                "shasum": ""

    This doesn't look right, this shouldn't be changing. 

  2. +++ b/composer.lock
@@ -2325,16 +2337,16 @@
-            "version": "v1.12.0",
+            "version": "v1.14.0",

    Upgrading this here is out of scope I think 

  3. +++ b/composer.lock
@@ -3091,16 +3103,16 @@
-            "version": "v3.4.35",
+            "version": "v3.4.38",

    Same with this 

  4. +++ b/composer/Metapackage/CoreRecommended/composer.json
@@ -17,6 +17,8 @@
+        "drupal/core-project-message": "8.9.x-dev",
+        "drupal/core-vendor-hardening": "8.9.x-dev",

    This doesn't look right either.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
StatusFileSize
new 3121885-7-8.8.x.patch3.74 KB
new 3121885-7-8.9.x.patch3.76 KB
new 3121885-7-9.0.x.patch2.19 KB

Thank you @longwave for reviewing and sharing to me the command below on slack

$ composer update drupal/coder squizlabs/php_codesniffer

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Related issues: +#3122122: Allow packages to get sorted
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Related issues: +#3096546: Add sorting configuration to composer.json
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Needs work

I've run phpcs with this patch on 9.0.x and 8.9.x. 9.0.x is fine but we need to fix 2 new coding standards issues on D8.9...

FILE: ...v/core/tests/Drupal/KernelTests/Core/Entity/EntitySchemaTest.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 331 | WARNING | [x] Empty PHP statement detected: superfluous
     |         |     semi-colon.
     |         |     (Generic.CodeAnalysis.EmptyPHPStatement.SemicolonWithoutCodeDetected)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...x/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/DrupalKernel.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 494 | WARNING | [x] Empty PHP statement detected: superfluous
     |         |     semi-colon.
     |         |     (Generic.CodeAnalysis.EmptyPHPStatement.SemicolonWithoutCodeDetected)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 2 mins, 26.11 secs; Memory: 164MB
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Needs work » Needs review
StatusFileSize
new 3121885-8.9.x-11.patch4.96 KB
new interdiff-7-11.txt1.2 KB

Thanks, @alexpott!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

#10 was addressed so this looks ready to go.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

@longwave, Thank you!

I've run phpcs with this patch on 9.0.x and 8.9.x. 9.0.x is fine but we need to fix 2 new coding standards issues on D8.9...

And one more question for @alexpott, how did you found that? Frankly, I did not reproduce that on my local and DrupalCI did not detect out them too, https://www.drupal.org/pift-ci-job/1626085 (from #7)

So could you share to me the steps or commands you run if it's not time-consuming? Your time is precious, do not want to take much your time. I could still try to figure it out myself. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@jungle on 8.9.x after applying #7 and running composer install, I ran the following command: composer run phpcs -- -ps. Core's root composer.json has a command that sets up phpcs properly and uses the correct phpcs.xml etc.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

@alexpott many thanks!

Reproduced #10 with your command composer run phpcs -- -ps

But one thing makes me confusing, and can't find an explanation from the documentation of phpcs

From the console, the actual command was

> phpcs --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer -- '-ps'

In composer.json

        "phpcs": "phpcs --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer --",

From my understanding, composer run phpcs -- -ps should be expended to

> phpcs --standard=core/phpcs.xml.dist --runtime-set installed_paths $($COMPOSER_BINARY config vendor-dir)/drupal/coder/coder_sniffer -- -- '-ps'

Remove the tail -- from the command in composer.json, it seems working too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed f7e6636652 to 9.1.x and 2e30f2c179 to 9.0.x. Thanks!

Committed 6a83036 and pushed to 8.9.x. Thanks!

As a dev only dependency this is fine to update.

  • alexpott committed f7e6636 on 9.1.x 
    Issue #3121885 by jungle, longwave, alexpott: Update coder to 8.3.8

  • alexpott committed 2e30f2c on 9.0.x 
    Issue #3121885 by jungle, longwave, alexpott: Update coder to 8.3.8

(...

  • alexpott committed 6a83036 on 8.9.x 
    Issue #3121885 by jungle, longwave, alexpott, cilefen: Update coder to 8...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.