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
I've separated this out from #1533096: Make includes directory, files starting with D-G pass Coder Review because there seem to be lots of fiddly issues, whereas all the other files are pretty tidy
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
Comment | File | Size | Author |
---|---|---|---|
#13 | 2054345-13.patch | 4.25 KB | sergeypavlenko |
#7 | 2054345-7.patch | 7.08 KB | sergeypavlenko |
#4 | form.inc-coder-2054345-4.patch | 20.65 KB | sphism |
Comments
Comment #1
sphism CreditAttribution: sphism commentedComment #2
sphism CreditAttribution: sphism commentedComment #3
sphism CreditAttribution: sphism commentedI'm going to work on this now and post a patch soon
Comment #4
sphism CreditAttribution: sphism commentedHere's a patch which improves form.inc, doesn't fix 100% of errors reported, but it's a good start, this is what coder reports after applying the patch:
So many of the errors require rewriting function calls and/or arrays across multiple lines, I think this is better but honestly I think a lot of it should just be written in a different style, for example:
Comment #5
AjitS4: form.inc-coder-2054345-4.patch queued for re-testing.
Comment #7
sergeypavlenko CreditAttribution: sergeypavlenko commentedHi
I removed all the unnecessary change lines. Making actual patch for the current version of Drupal 8.
Comment #8
sergeypavlenko CreditAttribution: sergeypavlenko commentedChange assigned.
Comment #9
dcam CreditAttribution: dcam commentedUpdating in preparation for DrupalCon Austin sprints. See http://www.hook42.com/blog/prepping-drupalcon-austin-sprints-sprint-lead....
Comment #10
sergeypavlenko CreditAttribution: sergeypavlenko commentedHi
Why here need manual tests? If the code has not been edited.
Comment #11
dcam CreditAttribution: dcam commentedGood point. It just needs reviewing, not actual testing.
Comment #12
Jalandhar CreditAttribution: Jalandhar commentedPatch #7 needs reroll.
Comment #13
sergeypavlenko CreditAttribution: sergeypavlenko commentedHi
I'm rerilled patch.
Comment #16
apratt CreditAttribution: apratt commentedMuch of this patch involves deprecated code now and the new code appears to be coder compliant. I suspect this issue could be closed.
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
xjmComment #19
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.
Comment #20
pfrenssen