Running coder results in a number of "red" warnings that are false positives, plus a fair number of "green" coding style issues. These are primarily:

  1. White space issues, particularly after foreach, around the dot (.) operator, and before the opening brace ({).
  2. Missing / incomplete @file docblocks.
  3. Trailing commas in one-line array -- e.g. array('a' => 1, 'b' => 2,).
  4. Spaces at the start or end of a translatable string. These fixes will have require re-translation, I believe, of the affected strings.
CommentFileSizeAuthor
#2 webform-coder_style_imperfections-2487748-2.patch20.19 KBdanchadwick

Comments

  • DanChadwick committed 33ee6b1 on 7.x-4.x 
    Issue #2487748 by DanChadwick: Coder review reveals whitespace and other...
danchadwick’s picture

danchadwick commented
Status: Active » Fixed
StatusFileSize
new webform-coder_style_imperfections-2487748-2.patch20.19 KB

Committed to 7.x-4.x.

danchadwick’s picture

danchadwick commented
Version: 7.x-4.x-dev » 8.x-4.x-dev
Category: Bug report » Task
Status: Fixed » Active

The 8.x branch should run coder at some point. I don't think porting this patch would be a good use of time.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
commented
Version: 8.x-4.x-dev » 7.x-4.x-dev
Category: Task » Bug report
Status: Active » Fixed

Committed and pushed to 8.x-4.x.

Half way through a bunch of substantial changes in the 8.x branch, however I decided to port this before finishing those. It wasn't too tricky which made a nice change!

  • fenstrat committed c132e36 on 8.x-4.x 
    Issue #2487748 by DanChadwick, fenstrat: Coder review reveals whitespace...
danchadwick’s picture

danchadwick commented

OCD. It's contagious. :)

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
commented

Haaa, that it is!

roball’s picture

roball commented
Status: Fixed » Needs work

There are still some problems with the usage of t(). See the 13 Source code warnings at https://localize.drupal.org/translate/projects/webform/releases/438008 for details of Webform 7.x-4.8.

fenstrat’s picture

fenstrat
he/him
Primary language English
Location Australia
commented
Status: Needs work » Fixed

@roball This issue is about minor code style fixes. Can you start a new issue for the t() fixes, thanks.

danchadwick’s picture

danchadwick commented

Please suggest solutions because I'm not at all sure that any of those t() issues are avoidable or reasonable. Just because coder says something doesn't make it true.

roball’s picture

roball commented

OK, here is the new issue: #2488930: Interface text translatability warnings. You can see these warnings as part of the Coder Review module's code review when you have the Translation template extractor 7.x-1.x module installed.

roball’s picture

roball commented

@DanChadwick sorry I have to disagree. All 13 source code warnings for Webform 7.x-4.8 are actually true. In fact, they are even errors since the affected 13 strings can never be translated using Drupal's .po files.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.