Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of meta-issue #1518116: [meta] Make Core pass Coder Review
- The Drupal 8 branch test code review tab is not available to check at the moment.
- Drupal Code Sniffer reports no errors but the missing directive types, as expected.
- Coder Review finds no problems at the minor (strictest) severity warning level.
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Reroll the patch if it no longer applies. | Novice | Instructions |
Comments
Comment #1
TravisCarden CreditAttribution: TravisCarden commentedComment #2
Lars Toomre CreditAttribution: Lars Toomre commentedThe #suffix key needs to be on its own line here.
Comment #3
TravisCarden CreditAttribution: TravisCarden commentedAh! Good catch; thank you.
Comment #4
TravisCarden CreditAttribution: TravisCarden commentedHere's an updated patch without the directive types, per the new direction of the larger initiative.
Comment #5
TravisCarden CreditAttribution: TravisCarden commentedHad to re-roll for upstream changes. Note, this patch is unaffected by #1539712: [policy, no patch] Coding standards for breaking function calls and language constructs across lines.
Comment #6
Lars Toomre CreditAttribution: Lars Toomre commented#5: comment-coder-fixes-5940134.patch queued for re-testing.
Comment #8
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 #9
sphism CreditAttribution: sphism commentedWe have the go ahead with all these issues again, see #1518116: [meta] Make Core pass Coder Review for more details
Comment #10
sandergo90 CreditAttribution: sandergo90 commentedI've tried to create all the correct coding standards. Attached the patch.
Comment #12
sandergo90 CreditAttribution: sandergo90 commentedAttached new patch for coding error.
Comment #13
sandergo90 CreditAttribution: sandergo90 commentedComment #15
sandergo90 CreditAttribution: sandergo90 commentedNew patch again.
Comment #17
sandergo90 CreditAttribution: sandergo90 commentedSorry for all the patches. Here's one again.
Comment #17.0
sandergo90 CreditAttribution: sandergo90 commentedUpdated issue summary.
Comment #18
parthipanramesh CreditAttribution: parthipanramesh commentedsorry but patch does not apply..
Comment #19
willieseabrook CreditAttribution: willieseabrook commentedStarting as part of SprintWeekend2014
Comment #20
deepakaryan1988I have created patch for it.
Comment #21
willieseabrook CreditAttribution: willieseabrook commentedComment #22
TravisCarden CreditAttribution: TravisCarden commentedA few things here:
Please address these issues so someone can productively perform a closer code review. Thanks!
Comment #23
roderikIn the meantime, many functions have been removed from comment.module so that will make this patch smaller. (In addition to taking out the "@param" things that are in #1808478: Add missing type hinting to Comment module docblocks now, as comment#22 said.) Find me if there are questions about comment.module stuff.
Rerolling this and seeing what is left of the patch after that, should be a novice issue.
Comment #24
Tomefa CreditAttribution: Tomefa commentedRerolling the patch at Drupalcon Amsterdam
Comment #25
Tomefa CreditAttribution: Tomefa commentedHere is the reroll of the patch.
I didn't resolve any coding standard issue, just rerolling it and make it work for the actual branch 8.0.x
Comment #26
roderikThanks for the reroll. But it is still 'needs work' to address the comments in #22
Comment #27
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 #28
pfrenssenClosing 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.