Features generate code like:

/*
* Implementation of hook_foo().
*/

where it according to the coding standard (http://drupal.org/node/1354#hookimpl) should be:

/*
* Implements hook_foo().
*/

Although it might not seem very important it makes the coder module complain about the code generated by features.

Attached is a patch (features.export.patch) to make features generate code according to coding standard regarding this.

Attached is also a patch (features.patch) to make features itself adhere to coding standard regarding this.

CommentFileSizeAuthor
#11 features-coding-standards-1231118-11.patch573 bytesmariacha1
#7 features-coding-standards-1231118-7.patch994 bytesnielsonm
#4 features-coding-standards-1231118-04.patch720 bytesjhedstrom
#1 features-1231118-1.patch20.47 KBtim.plunkett
features.patch22.41 KBarnested
features.export.patch677 bytesarnested

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 7.x-1.0-beta3 » 7.x-1.x-dev
StatusFileSize
new features-1231118-1.patch20.47 KB

Rolled a proper -p1 patch.

dead_arm’s picture

dead_arm
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Patch looks fine.

febbraro’s picture

febbraro commented
Status: Reviewed & tested by the community » Closed (fixed)

Thanks.

http://drupalcode.org/project/features.git/commit/82ac8d7

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented
Status: Closed (fixed) » Needs review
StatusFileSize
new features-coding-standards-1231118-04.patch720 bytes

I found one more issue (regarding usage of include_once), when using Drupal Code Sniffer on a features-generated module. Very simple fix attached.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Fixed

Fixed in both branches.
http://drupalcode.org/project/features.git/commit/a9f9f92
http://drupalcode.org/project/features.git/commit/83e0a6c

Status: Fixed » Closed (fixed)

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

nielsonm’s picture

nielsonm commented
StatusFileSize
new features-coding-standards-1231118-7.patch994 bytes

I noticed the inline comments for exported user roles and permissions lacked a full stop, so here's a patch.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Closed (fixed) » Reviewed & tested by the community
mpotter’s picture

mpotter commented
Status: Reviewed & tested by the community » Fixed

Committed http://drupalcode.org/project/features.git/commit/c04fce6.

hefox’s picture

hefox commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Fixed » Patch (to be ported)
mariacha1’s picture

mariacha1 commented
Version: 6.x-1.x-dev » 7.x-2.x-dev
Status: Patch (to be ported) » Needs review
StatusFileSize
new features-coding-standards-1231118-11.patch573 bytes

One more patch that fixes a double-spacing issue at the bottom of the hook_menu_default_menu_links() implementation. If I have any translatables, I get the error:

Functions must not contain multiple empty lines in a row; found 2 empty lines

Attached is a patch.

hefox’s picture

hefox commented
Issue summary: View changes

Probably still an issue?

  • febbraro committed 82ac8d7 on 8.x-3.x 
    Issue #1231118 by arnested, tim.plunkett: Coder fixes
  • tim.plunkett committed 83e0a6c on 8.x-3.x authored by jhedstrom
    Issue #1231118 by jhedstrom: Generate code according to coding standards...
  • mpotter committed c04fce6 on 8.x-3.x
    Issue #1231118 by tim.plunkett, jhedstrom, nielsonm, arnested: Fixed...
mpotter’s picture

mpotter commented
Status: Needs review » Reviewed & tested by the community

bedlam queued 11: features-coding-standards-1231118-11.patch for re-testing.

bedlam’s picture

bedlam
Location Toronto
commented

+1 for the patch in #11:

  • It's still an issue.
  • The patch still tests ok.
  • The fix is correct: moving a linebreak to just before the call to features_translatables_export() makes sense because:
mpotter’s picture

mpotter commented
Status: Reviewed & tested by the community » Fixed

Committed #11 to 38b22da.

  • mpotter committed 38b22da on 7.x-2.x authored by mariacha1 
    Issue #1231118 by arnested, tim.plunkett, nielsonm, jhedstrom, mariacha1...

Status: Fixed » Closed (fixed)

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

arnested’s picture

arnested commented