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
StatusFileSize
new32.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

StatusFileSize
new32.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
StatusFileSize
new31.04 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,632 pass(es).
[ View ]
new7.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.