Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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 --------------------------------------------------------------------------------
Comment | File | Size | Author |
---|---|---|---|
#8 | translation_standardized-1533440-5918390.patch | 8.49 KB | FluxSauce |
#2 | translation_standardized-1533440.patch | 15.5 KB | FluxSauce |
Comments
Comment #1
FluxSauce CreditAttribution: FluxSauce commentedComment #2
FluxSauce CreditAttribution: FluxSauce commentedCompleted.
In case there's any question, in translation.module I switched
COUNT(*)
toCOUNT(nid)
, the primary key because ">coder correctly flagged it as a warning.Comment #2.0
FluxSauce CreditAttribution: FluxSauce commentedReport of completed work and false positives
Comment #3
dinakaran.ilango CreditAttribution: dinakaran.ilango commentedpatch at #2 works for me
Comment #4
lotyrin CreditAttribution: lotyrin commentedThis patch is adding types to documentation blocks. AFAIK, that's supposed to be part of a separate meta issue.
Comment #5
Lars Toomre CreditAttribution: Lars Toomre commentedYup.. 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.
Comment #6
jhodgdonWe'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.
Comment #7
FluxSauce CreditAttribution: FluxSauce commentedI 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).
Comment #7.0
FluxSauce CreditAttribution: FluxSauce commentedMinor markup cleanup
Comment #8
FluxSauce CreditAttribution: FluxSauce commentedResubmitting without type hinting.
Comment #9
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedAll 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.
Comment #10
FluxSauce CreditAttribution: FluxSauce commentedYes. I listed the errors that are still reported in the issue description. To summarize:
Comment #11
TravisCarden CreditAttribution: TravisCarden commentedPostponing till feature freeze. If you want to help in the meantime, please work on the blockers on the meta issue. Thanks!
Comment #12
FluxSauce CreditAttribution: FluxSauce commentedComment #12.0
FluxSauce CreditAttribution: FluxSauce commentedUpdating with revised patch results.
Comment #13
TravisCarden CreditAttribution: TravisCarden commentedComment #14
valthebaldWe are going to work on this issue during DCWroclaw 2014
Comment #17
xjmThanks 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.
Comment #18
tatarbjClosing in favor of #2571965: [meta] Fix PHP 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.