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
Comment | File | Size | Author |
---|---|---|---|
#7 | drupal-code_cleanup-1533234-7.patch | 25.88 KB | -enzo- |
#5 | drupal-code_cleanup-1533234-5.patch | 26.13 KB | -enzo- |
#2 | drupal-code_cleanup-1533234-2.patch | 25.96 KB | -enzo- |
Comments
Comment #1
scottalan CreditAttribution: scottalan commentedComment #2
-enzo- CreditAttribution: -enzo- commentedHey folks
I did some code cleanup included in the patch attached.
But there are some ERRORS reported by drupalcs, I don't know how to resolve
24 | ERROR | Missing function doc comment
24 | ERROR | No scope modifier specified for function "setUp"
-- see http://drupal.org/node/1545476 false positive "Missing function doc comment" error on Simple Test inherited methods
55 | ERROR | No scope modifier specified for function "testDefaultWidgetPropertiesAlter"
199 | ERROR | Do not use t() in hook_menu()
Also I can't excecute coder against drupal 8 modules. if you have any idea how to this, I will really appreciate your comments.
Comment #3
-enzo- CreditAttribution: -enzo- commentedI did the changes following the recommendations of drupalcs with this patch http://drupal.org/node/1518116#comment-6027004
Here the definition of my drupalcs alias
phpcs --standard=$HOME/drupalcs/Drupal/ruleset.xml --extensions=php,module,inc,install,test,profile,theme'
Comment #4
tim.plunkettThis should be split over multiple lines, or none at all
I don't think this is in our coding standard.
I'd join these two together
I'd join these two together
Should end with a comma and move the ), to the next line
Comment #5
-enzo- CreditAttribution: -enzo- commentedHello tim
Thanks for your feedback , I did the changes I understand, but I dont' understand your comments
1)
2)
If you can use this new patch and create an interdiff I will really appreciate to learn how to apply in other code clean I want to do.
Regards,
Comment #6
tim.plunkett1)
I meant like
2) We don't usually break up function calls over multiple lines, it should just be left as is.
Comment #7
-enzo- CreditAttribution: -enzo- commentedThanks tim
I did the last changes you recommend, I guess now this patch needs a community revision.
Comment #8
bleen CreditAttribution: bleen commentedThis looks wrong. I think that only "settings)." should be on the new line
Comment #9
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 #10
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 #11
dcmul CreditAttribution: dcmul commentedam working on this right now.
Comment #12
swentel CreditAttribution: swentel commentedThere is a duplicate at #1533232: Make field module pass Coder Review