Part of meta-issue #1518116: [meta] Make Core pass Coder Review

  • Failed Drupal 8 branch test code review tab (one critical reported in core/modules/translation/translation.module)
  • No errors or warnings reported using Coder 8
    • Minor: Drupal Coding Standards, Drupal Commenting Standards, Drupal SQL Standards, Drupal Security Checks, Internationalization
  • Multiple false positives reported using Drupal Code Sniffer
    • phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme
    • The "Missing parameter type" and "Data type of return value" errors are considered out of scope as per #1518116: [meta] Make Core pass Coder Review. The "Missing function doc comment" errors are currently considered a false positive, reported in #1545476: false positive "Missing function doc comment" error on Simple Test inherited methods.
    • FILE: /media/sf_sandbox/core/modules/translation/translation.module
      --------------------------------------------------------------------------------
      FOUND 8 ERROR(S) AFFECTING 8 LINE(S)
      --------------------------------------------------------------------------------
        80 | ERROR | Missing parameter type at position 1
        83 | ERROR | Data type of return value is missing
      400 | ERROR | Missing parameter type at position 1
      437 | ERROR | Missing parameter type at position 1
      441 | ERROR | Data type of return value is missing
      471 | ERROR | Data type of return value is missing
      481 | ERROR | Missing parameter type at position 1
      484 | ERROR | Data type of return value is missing
      --------------------------------------------------------------------------------
      FILE: /media/sf_sandbox/core/modules/translation/translation.pages.inc
      --------------------------------------------------------------------------------
      FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
      --------------------------------------------------------------------------------
      16 | ERROR | Data type of return value is missing
      --------------------------------------------------------------------------------
      FILE: /media/sf_sandbox/core/modules/translation/translation.test
      --------------------------------------------------------------------------------
      FOUND 26 ERROR(S) AFFECTING 26 LINE(S)
      --------------------------------------------------------------------------------
        18 | ERROR | Missing function doc comment
        26 | ERROR | Missing function doc comment
      268 | ERROR | Missing parameter type at position 1
      271 | ERROR | Data type of return value is missing
      281 | ERROR | Missing parameter type at position 1
      312 | ERROR | Missing parameter type at position 1
      314 | ERROR | Missing parameter type at position 2
      316 | ERROR | Missing parameter type at position 3
      319 | ERROR | Data type of return value is missing
      345 | ERROR | Missing parameter type at position 2
      347 | ERROR | Missing parameter type at position 3
      349 | ERROR | Missing parameter type at position 4
      352 | ERROR | Data type of return value is missing
      380 | ERROR | Missing parameter type at position 1
      386 | ERROR | Missing parameter type at position 3
      388 | ERROR | Missing parameter type at position 4
      390 | ERROR | Missing parameter type at position 5
      393 | ERROR | Data type of return value is missing
      404 | ERROR | Missing parameter type at position 1
      406 | ERROR | Missing parameter type at position 2
      408 | ERROR | Missing parameter type at position 3
      410 | ERROR | Missing parameter type at position 4
      413 | ERROR | Data type of return value is missing
      462 | ERROR | Missing parameter type at position 1
      468 | ERROR | Missing parameter type at position 3
      471 | ERROR | Data type of return value is missing
      --------------------------------------------------------------------------------
Files: 
CommentFileSizeAuthor
#8 translation_standardized-1533440-5918390.patch8.49 KBFluxSauce
PASSED: [[SimpleTest]]: [MySQL] 36,487 pass(es).
[ View ]
#2 translation_standardized-1533440.patch15.5 KBFluxSauce
PASSED: [[SimpleTest]]: [MySQL] 35,421 pass(es).
[ View ]

Comments

Assigned:Unassigned» FluxSauce

Status:Active» Needs review
StatusFileSize
new15.5 KB
PASSED: [[SimpleTest]]: [MySQL] 35,421 pass(es).
[ View ]

Completed.

In case there's any question, in translation.module I switched COUNT(*) to COUNT(nid), the primary key because coder correctly flagged it as a warning.

Issue summary:View changes

Report of completed work and false positives

Status:Needs review» Reviewed & tested by the community

patch at #2 works for me

Status:Reviewed & tested by the community» Needs work

This patch is adding types to documentation blocks. AFAIK, that's supposed to be part of a separate meta issue.

Yup.. Adding type hinting is too hard for this sprint of patches. The committer is too concerned about enough time to correctly review all of the functions in this module.

We're not including the param/return types in this effort any more -- see #1518116: [meta] Make Core pass Coder Review -- too difficult to review for accuracy.

I will re-do the patch without the type hinting.

For the record, the stipulation that type hinting was to be excluded was made after I wrote the patch (24/04/2012 - 21:33).

Issue summary:View changes

Minor markup cleanup

Status:Needs work» Needs review
StatusFileSize
new8.49 KB
PASSED: [[SimpleTest]]: [MySQL] 36,487 pass(es).
[ View ]

Resubmitting without type hinting.

All of the changes look good but I didn't apply the patch. Does it pass with the exception of the datatype errors produced? If it doesn't can you say what errors are still produced.

Does it pass with the exception of the datatype errors produced? If it doesn't can you say what errors are still produced.

Yes. I listed the errors that are still reported in the issue description. To summarize:

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!

Assigned:FluxSauce» Unassigned

Issue summary:View changes

Updating with revised patch results.

Status:Postponed» Active