Files: 
CommentFileSizeAuthor
#7 drupal.make-statistics-module-pass-coder-review.1533390-07.patch19.62 KBvisabhishek
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,588 pass(es).
[ View ]
#6 drupal-make_statistics_module_pass_coder_review-1533390-06.patch19.75 KBvisabhishek
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,426 pass(es).
[ View ]
#2 drupal-statistics-coding-standards-1533390-2.patch36.63 KBtheduke
PASSED: [[SimpleTest]]: [MySQL] 41,238 pass(es).
[ View ]

Comments

Status:Active» Postponed

Assigned:Unassigned» theduke
StatusFileSize
new36.63 KB
PASSED: [[SimpleTest]]: [MySQL] 41,238 pass(es).
[ View ]

I'm taking this on.

Patch attached.

Coder reports no problems.

Code Sniffer reports several errors, but I consider all of those to be false positives/invalid:

FILE: .../modules/statistics/lib/Drupal/statistics/Tests/StatisticsAdminTest.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
185 | ERROR | Using the e flag in preg_match is a possible security risk. For
     |       | details see http://drupal.org/node/750148
--------------------------------------------------------------------------------

This seems to be an error in Drupal Code Sniffer.
See http://drupal.org/node/1779020

FILE: .../drupal8.local/public_html/core/modules/statistics/statistics.admin.inc
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
318 | ERROR | If the line declaring an array spans longer than 80 characters,
     |       | each element should be broken into its own line
--------------------------------------------------------------------------------

In my opinion, changing this would not do anything for readability. It would actually harm it.

FILE: ...dev/drupal8.local/public_html/core/modules/statistics/statistics.module
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
  32 | ERROR | If the line declaring an array spans longer than 80 characters,
     |       | each element should be broken into its own line
327 | ERROR | If the line declaring an array spans longer than 80 characters,
     |       | each element should be broken into its own line
--------------------------------------------------------------------------------

Line 32: All the output in the top is difficult to make readable, but I think it's alright the way it is.
Line 327: Same as previous file.

Status:Postponed» Needs review

Status:Needs review» Postponed

Postponing till feature freeze. If you want to help in the meantime, please work on the blockers on the meta issue. Thanks!

Status:Postponed» Active

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

Assigned:theduke» visabhishek
Issue summary:View changes
Status:Active» Needs review
StatusFileSize
new19.75 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,426 pass(es).
[ View ]

I have created a patch. Please review.

StatusFileSize
new19.62 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,588 pass(es).
[ View ]

I have updated this patch as per https://drupal.org/comment/8601579#comment-8601579.