There's few core coding standards violations should be fixed before module can go to core

CommentFileSizeAuthor
#8 2920498-8.patch14.27 KBandypost
#6 2920498-6.patch8.14 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost created an issue. See original summary.

andypost’s picture

After #2920479: Get rid of rendering in hook_help

PHP CODE SNIFFER REPORT SUMMARY
----------------------------------------------------------------------
FILE                                                  ERRORS  WARNINGS
----------------------------------------------------------------------
...odules/config_help_test/config_help_test.info.yml  0       1
/work/tests/src/Functional/HelpTopicTest.php          3       1
/work/tests/src/Functional/HelpTopicAdminTest.php     19      0
...k/tests/src/Functional/HelpTopicTranslateTest.php  10      0
/work/tests/src/Kernel/HelpTopicTokensTest.php        1       0
/work/tests/src/Kernel/TextSectionRenderingTest.php   1       0
/work/config_help.tokens.inc                          1       0
/work/src/Entity/HelpTopic.php                        1       0
/work/src/HelpAccessControlHandler.php                1       0
/work/src/Form/HelpTopicForm.php                      1       1
/work/src/Element/TextSections.php                    1       0
/work/src/TextSectionManager.php                      2       0
/work/src/TextSectionPluginCollection.php             2       0
/work/src/HelpListBuilder.php                         3       0
/work/src/Plugin/HelpSection/ConfigHelpSection.php    0       2
/work/src/Controller/AutocompleteController.php       3       0
/work/src/HelpViewBuilder.php                         1       0
/work/src/HelpTopicInterface.php                      4       0
----------------------------------------------------------------------
A TOTAL OF 54 ERRORS AND 5 WARNINGS WERE FOUND IN 18 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 33 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
jhodgdon’s picture

Assigned: Unassigned » jhodgdon
Related issues: +#2920309: Add experimental module for Help Topics

Good! I'll take a look at the test bot output in detail and make a patch here.

jhodgdon’s picture

Besides the sniffs, let's also fix this review from comment #13 on the related issue here:

+++ b/core/modules/config_help/src/TextSectionPluginCollection.php
@@ -0,0 +1,41 @@
+    $body = $help_topic->getBody();
+    if (!empty($body)) {

if ($body) should be enough

jhodgdon’s picture

Also fixing this one here:

+++ b/core/modules/config_help/src/Entity/HelpTopic.php
@@ -0,0 +1,331 @@
+   * @var array
...
+  protected $body;

Looks it needs default value - empty array

jhodgdon’s picture

Status: Active » Needs review
FileSize
8.14 KB

Here's a patch for the specific things here, as well as I hope the Coder warnings/errors from the test bot.

The config_help tests passed locally, so I'm committing it to the Sandbox now, and will shortly updating the Core patch on #2920309-23: Add experimental module for Help Topics. After that test bot run, we can see if there are more fixes needed to get rid of all Coder warnings. Until then, leaving this at Needs Review.

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Status: Needs review » Fixed

The latest tests on the bot show no coding standards problems, so I think I will go ahead and close this as Fixed.

Not sure if it does warnings or only errors... if there are warnings we should fix, we can reopen this issue.

andypost’s picture

Status: Fixed » Needs review
FileSize
14.27 KB

Core sniffers are limited so here is fixes from DrupalPractice & core remains that still in work #2571965: [meta] Fix PHP coding standards in core

andypost’s picture

jhodgdon’s picture

Status: Needs review » Fixed

Thanks, this all looks great to me. Committed to Sandbox and I'm updating the Core patch as well.

  • jhodgdon committed f6f5ede on 8.x-1.x
    Issue #2920498 by andypost, jhodgdon: Fix coding standards issues
    

  • jhodgdon committed 8ec912b on 8.x-1.x
    Issue #2920498 by andypost: Fix coding standards issues
    

Status: Fixed » Closed (fixed)

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