Files: 
CommentFileSizeAuthor
#15 interdiff-1533240-12-15.txt7.18 KBpguillard
#15 make_forum_module_pass_coder_review-1533240-15.patch31.04 KBpguillard
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,632 pass(es). View
#12 make_forum_module_pass_coder_review-1533240-12.patch32.1 KBpguillard
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,488 pass(es). View
#10 make_forum_module_pass_coder_review-1533240-10.patch32.15 KBpguillard
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/src/Tests/ForumTest.php. View

Comments

larowlan’s picture

Assigned: Unassigned » larowlan
NROTC_Webmaster’s picture

Status: Active » Postponed
sphism’s picture

Status: Postponed » Active

We have the go ahead with all these issues again, see #1518116: [meta] Make Core pass Coder Review for more details

larowlan’s picture

Status: Active » Postponed

This should wait for #1951268: Convert /forum and /forum/% to new router, remove forum_forum_load(), forum_get_topics(), create Forum service and #1974210: Convert admin/structure/forum to a Controller
forum.pages.inc and forum.admin.inc will be removed in those issues.
And forum.module will contain hooks only, no other code.

larowlan’s picture

Assigned: larowlan » Unassigned
mgifford’s picture

cs_shadow’s picture

Working on it.

pguillard’s picture

I take this one

pguillard’s picture

Assigned: Unassigned » pguillard
pguillard’s picture

Status: Active » Needs review
FileSize
32.15 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/forum/src/Tests/ForumTest.php. View

Here is a patch.

Remaining errors are :
$phpcs --standard=Drupal core/modules/forum

FILE: ...s/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
5 | WARNING | Line exceeds 80 characters; contains 82 characters
--------------------------------------------------------------------------------

FILE: /var/www/drupal8/core/modules/forum/src/ForumManager.php
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
125 | WARNING | Line exceeds 80 characters; contains 83 characters
--------------------------------------------------------------------------------

Status: Needs review » Needs work

The last submitted patch, 10: make_forum_module_pass_coder_review-1533240-10.patch, failed testing.

pguillard’s picture

FileSize
32.1 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,488 pass(es). View

Correction

mgifford’s picture

Status: Needs work » Needs review
larowlan’s picture

Status: Needs review » Needs work

This looks great! a few minor changes required

  1. +++ b/core/modules/forum/src/ForumManager.php
    @@ -22,10 +22,13 @@
    +  // A false positive error on that lines.
    

    » A false positive error on these lines.

  2. +++ b/core/modules/forum/src/ForumSettingsForm.php
    @@ -28,7 +28,27 @@ public function getFormId() {
    -    $options = array(5, 10, 15, 20, 25, 30, 35, 40, 50, 60, 80, 100, 150, 200, 250, 300, 350, 400, 500);
    +    $options = array(
    +      5,
    +      10,
    +      15,
    +      20,
    +      25,
    +      30,
    +      35,
    +      40,
    +      50,
    +      60,
    +      80,
    +      100,
    +      150,
    +      200,
    +      250,
    +      300,
    +      350,
    +      400,
    +      500,
    +    );
    

    does this pass review if you split it to 5 per line, the readability here isn't great

  3. +++ b/core/modules/forum/src/Tests/ForumIndexTest.php
    @@ -23,18 +23,27 @@ class ForumIndexTest extends WebTestBase {
    +        'edit any forum content','administer nodes',
    

    should be on own line?

  4. +++ b/core/modules/forum/src/Tests/ForumTest.php
    @@ -316,7 +332,7 @@ private function doAdminTests($user) {
    +  private function editForumVocabulary() {
    
    @@ -400,10 +420,10 @@ function createForum($type, $parent = 0) {
    +  public function deleteForum($tid) {
    
    @@ -475,13 +495,13 @@ function testForumWithNewPost() {
    +  private function createForumTopic($forum, $container = FALSE) {
    

    please use protected instead

pguillard’s picture

Status: Needs work » Needs review
FileSize
31.04 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,632 pass(es). View
7.18 KB

Thanks. Changes have been applied.

xjm’s picture

Version: 8.0.x-dev » 8.1.x-dev
Assigned: pguillard » Unassigned
Priority: Normal » Minor
Status: Needs review » Postponed
Issue tags: -Novice

Thanks for all the work here so far. See #1518116-86: [meta] Make Core pass Coder Review. This issue is postponed until the meta issue is either closed or reopened.

pfrenssen’s picture

Status: Postponed » Closed (duplicate)

Closing in favor of #2571965: [meta] Fix coding standards in core. In this issue the coding standards will be fixed on a sniff-per-sniff basis rather than a module-per-module basis.