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
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Update the issue summary | Yes | Instructions | |
Add automated tests | Instructions | ||
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
Comments
Comment #1
nick_schuch CreditAttribution: nick_schuch commentedFirst attempt.
Comment #2
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedUnfortunately, you shouldn't make any changes to the docblocks because #1313980: Clean up API docs for node module is still in progress. At a minimum this will need a reroll after the other issue is fixed. I would suggest picking another issue to work on until then.
Comment #3
Lars Toomre CreditAttribution: Lars Toomre commentedThe issue #1313980: Clean up API docs for node module is mostly complete (except for a review of the Test classes).
As a result, it would be great if someone could post here at least the results of running this module through the D7 Coder with the appropriate settings. At present, I am unable to do so and would really like to see how close this key core module is to passing a full Coder review.
I think that we can then roll a revised patch addresses many of the flagged items. Thanks in advance.
Comment #4
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 #5
xjm(Merging "node system" and "node.module" components for 8.x; disregard.)
Comment #6
TravisCarden CreditAttribution: TravisCarden commentedComment #7
blueminds CreditAttribution: blueminds commentedplease see the patch.
There are still 5 minor warnings that come from test assertions like
$this->assertRaw('>' . $newterm1 . '<', 'First new term displayed.');
Comment #8
minneapolisdan CreditAttribution: minneapolisdan commentedComment #9
blueminds CreditAttribution: blueminds commentedPSR4 reroll
Comment #12
henk CreditAttribution: henk commentedPatch is rerolled, but I found also some nasty code in node.module line 107, this should be placed in template in my opinion:
Comment #13
henk CreditAttribution: henk commentedComment #14
henk CreditAttribution: henk commentedstatus to needs review
Comment #15
omessaoudi CreditAttribution: omessaoudi commentedI tried to apply the patch that does not pass. I'll try to rerol
Comment #16
omessaoudi CreditAttribution: omessaoudi commentedThis is the patch rerolled
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.