The larger effort to get all of Drupal core into compliance with Drupal's coding standards has been broken into the following issues:

Files: 
CommentFileSizeAuthor
#79 spartaaaa.png26.02 KBoriol_e9g
#74 drupal-1109202-74.patch38.17 KBTravisCarden
PASSED: [[SimpleTest]]: [MySQL] 35,930 pass(es).
[ View ]
#71 drupal-1109202-71.patch39.09 KBTravisCarden
PASSED: [[SimpleTest]]: [MySQL] 35,915 pass(es).
[ View ]
#59 1109202-59-coding-standards.patch158.2 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 32,939 pass(es).
[ View ]
#55 1109202-55-coding-standards.patch158.73 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] 32,941 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#49 1109202-49-coding-standards.patch134.69 KBklausi
PASSED: [[SimpleTest]]: [MySQL] 33,552 pass(es).
[ View ]
#42 1109202-42_coding_standards.patch136.92 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1109202-42_coding_standards.patch.
[ View ]
#37 1109202-37_coding_standards.patch129.76 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1109202-37_coding_standards.patch.
[ View ]
#34 1109202-34_coding_standards.patch130.6 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 30,038 pass(es).
[ View ]
#32 coding-standards2.patch130.6 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 30,004 pass(es).
[ View ]
#31 coding-standards.patch130.38 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 30,033 pass(es).
[ View ]
#28 newwwwww_one.patch90.59 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch newwwwww_one.patch.
[ View ]
#26 drupal-coding-standards_0_0.patch90.59 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards_0_0_0.patch.
[ View ]
#24 drupal-coding-standards_0.patch90.6 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards_0_0.patch.
[ View ]
#21 drupal-coding-standards.patch90.59 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 19,936 pass(es).
[ View ]
#19 drupal-coding-standards.patch90.59 KBdroplet
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards.patch.
[ View ]
#17 drupal-coding-standards-d8.patch90.59 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#14 code_stand.patch69.86 KBdroplet
PASSED: [[SimpleTest]]: [MySQL] 31,565 pass(es).
[ View ]
#12 coding-standard-love6.patch67.28 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,559 pass(es).
[ View ]
#13 coding-standard-love7.patch67.28 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,573 pass(es).
[ View ]
#10 drupal-coding-standards4.patch37.77 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,559 pass(es).
[ View ]
#9 drupal-coding-standards3.patch37.77 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,567 pass(es).
[ View ]
#3 coding-standard-love2.patch11.5 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,565 pass(es).
[ View ]
#1 coding-standard-love.patch11.5 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,539 pass(es).
[ View ]
drupal-conding-standards.patch6.18 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 31,544 pass(es).
[ View ]

Comments

StatusFileSize
new11.5 KB
PASSED: [[SimpleTest]]: [MySQL] 31,539 pass(es).
[ View ]

More fixes, more coding standard love.

Status:Needs review» Needs work

+++ b/includes/filetransfer/ssh.inc
@@ -72,7 +72,8 @@ class FileTransferSSH extends FileTransfer implements FileTransferChmodInterface
-    } else {
+    } ¶
+    else {

you are introducing trailing whitespaces in your else statements changes.

Status:Needs work» Needs review
StatusFileSize
new11.5 KB
PASSED: [[SimpleTest]]: [MySQL] 31,565 pass(es).
[ View ]

ok, trying again.

Patch in #3 looks good. How did you generate a list of these coding style bugs?

Status:Needs review» Reviewed & tested by the community

looks good now

@bfroehle with coder module: http://drupal.org/project/coder

Status:Reviewed & tested by the community» Needs work

it has 2 spaces between .= and t('After.....

       $output .=  t('After you create an image style, you can add effects: crop, scale, resize, rotate, and desaturate (other contributed modules provide additional effects). For example, by combining effects as crop, scale, and desaturate, you can create square, grayscale thumbnails.') . '<dd>';

Assigned:Unassigned» oriol_e9g

Status:Needs work» Needs review
StatusFileSize
new37.77 KB
PASSED: [[SimpleTest]]: [MySQL] 31,567 pass(es).
[ View ]

The two spaces was a mistake before & after de patch. I have found other double spaces around the code and I fixed it with a new patch.

StatusFileSize
new37.77 KB
PASSED: [[SimpleTest]]: [MySQL] 31,559 pass(es).
[ View ]

Ops! One more.

Status:Needs review» Needs work

More fixes comming

StatusFileSize
new67.28 KB
PASSED: [[SimpleTest]]: [MySQL] 31,559 pass(es).
[ View ]

Every loop it's a bigger patch :S

Status:Needs work» Needs review
StatusFileSize
new67.28 KB
PASSED: [[SimpleTest]]: [MySQL] 31,573 pass(es).
[ View ]

StatusFileSize
new69.86 KB
PASSED: [[SimpleTest]]: [MySQL] 31,565 pass(es).
[ View ]

catch few more SPACE issue.

Status:Needs review» Reviewed & tested by the community

Tests passes and patch #14 cleanly apply

Version:7.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Needs review

D8 first. Let's get these fixed on D8 and into D7.

StatusFileSize
new90.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ok. Trying again, D8 patch.

This should definitely go in soon assuming that it is still passing tests for D8.

Seeing this issue makes me wonder whether we should create a new issue to add 'tough love' coder SimpleTests to the standard core testing suite. Even if we were to start simple, we could automatically highlight the extra white-space errors, the missing blank line at the end of a file and other such common errors that seem to frequently pop up in the patch review and approval process.

Some of the other tests that could be easily include are tests for spacing around dot operators, space before opening brace of a function and double spaces in doc block comments, all of which seem to be the main items being caught here.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new90.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards.patch.
[ View ]

rename for testbots.

It's same as #17, so RTBC by me :)

it's a bit crazy to review it, hoping commit to core immediately :)

Status:Reviewed & tested by the community» Needs work

The last submitted patch, drupal-coding-standards.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new90.59 KB
PASSED: [[SimpleTest]]: [MySQL] 19,936 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

#19, I uploaded a wrong file.

#21 & #17 exactly same patch with rename, so RTBC.

--- a/modules/field_ui/field_ui.test
+++ b/modules/field_ui/field_ui.test
-    $type_name =  strtolower($this->randomName(8)) . '_' .'test';
+    $type_name = strtolower($this->randomName(8)) . '_' .'test';
...
-    $type_name2 =  strtolower($this->randomName(8)) . '_' .'test';
+    $type_name2 = strtolower($this->randomName(8)) . '_' .'test';

Note the last concatenation is missing a space.

--- a/modules/search/search-result.tpl.php
+++ b/modules/search/search-result.tpl.php
-    preg_match_all('/ (-?)("[^"]+"|[^" ]+)/i', ' ' .  $this->searchExpression , $keywords, PREG_SET_ORDER);
+    preg_match_all('/ (-?)("[^"]+"|[^" ]+)/i', ' ' . $this->searchExpression , $keywords, PREG_SET_ORDER);

There is stray whitespace after "$this->searchExpression".

I am leaving at RTBC though, because the 3 cases above aren't regressions, they are just additional coding standards problems unveiled by the patch
...and because this fixes about a bazillion coding standards problems in Drupal core
(and it passes tests and I double-checked that there were no code changes.)

EDIT: Fix formatting

StatusFileSize
new90.6 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards_0_0.patch.
[ View ]

okay. again.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, drupal-coding-standards_0.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new90.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-coding-standards_0_0_0.patch.
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-coding-standards_0_0.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new90.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch newwwwww_one.patch.
[ View ]

strange, the file is different than I upload... what's wrong..

#28: newwwwww_one.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, newwwwww_one.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new130.38 KB
PASSED: [[SimpleTest]]: [MySQL] 30,033 pass(es).
[ View ]

Again. Infinite perseverance.

StatusFileSize
new130.6 KB
PASSED: [[SimpleTest]]: [MySQL] 30,004 pass(es).
[ View ]

missed this one :)

-    preg_match_all('/ (-?)("[^"]+"|[^" ]+)/i', ' ' .  $this->searchExpression , $keywords, PREG_SET_ORDER);
+    preg_match_all('/ (-?)("[^"]+"|[^" ]+)/i', ' ' . $this->searchExpression, $keywords, PREG_SET_ORDER);

When patching the coding the standards, it will be nice to name the patches according to Drupal standards or at least recommandations by inserting the issue number and comment number to the beginning of patch filename.

If I post the follow-up patch in this comment, I will use the name 1109202-33_coding_standards.patch or similar.

Nice catches BTW.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new130.6 KB
PASSED: [[SimpleTest]]: [MySQL] 30,038 pass(es).
[ View ]

Only file name change.

Status:Reviewed & tested by the community» Needs review

Cool :-) the rename wasn't really necessary it is just very recommended to name the patches like this.

But you can't change the state to RTBC if you are the author or one of the authors of the patch... you need to wait until someone review it indepedently.

Status:Needs review» Needs work

+++ b/modules/system/system.tar.incundefined
+++ b/modules/system/system.tar.incundefined
@@ -94,10 +94,10 @@ class Archive_Tar // extends PEAR
     * If the compress argument is set the tar will be read or created as a
     * gzip or bz2 compressed TAR file.
     *
-    * @param    string  $p_tarname  The name of the tar archive to create
+    * @param    string  $p_tarname The name of the tar archive to create
     * @param    string  $p_compress can be null, 'gz' or 'bz2'. This
     *                   parameter indicates if gzip or bz2 compression
-    *                   is required.  For compatibility reason the
+    *                   is required. For compatibility reason the
     *                   boolean value 'true' means 'gz'.

You can't edit system.tar.inc because its not rly drupalcode. Its an extension someone outside drupal wrote. See the information on the top of this file.

If that part gets removed this is rdy to go (man thats a big patch...)

Powered by Dreditor.

Status:Needs work» Needs review
StatusFileSize
new129.76 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1109202-37_coding_standards.patch.
[ View ]

Okay. regenerate again:

git apply --exclude=modules/system/system.tar.inc
git diff > 1109202-37_coding_standards.patch

Status:Needs review» Reviewed & tested by the community
Issue tags:+coding standards

Looks good!

Status:Reviewed & tested by the community» Postponed

Marking this postponed until the # of criticals for D7 & D8 gets < 15.

Status:Postponed» Needs review
Issue tags:-coding standards

#37: 1109202-37_coding_standards.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+coding standards

The last submitted patch, 1109202-37_coding_standards.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new136.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1109202-42_coding_standards.patch.
[ View ]

Drupal 7.4 released
D7 & D8 criticals < 15
Try again

No reviews?

No reviews?

Status:Needs review» Reviewed & tested by the community

I'm back :)

Status:Reviewed & tested by the community» Needs work

@oriol_e9g,

see #36 and #37

Status:Needs work» Needs review
Issue tags:-coding standards

#42: 1109202-42_coding_standards.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+coding standards

The last submitted patch, 1109202-42_coding_standards.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new134.69 KB
PASSED: [[SimpleTest]]: [MySQL] 33,552 pass(es).
[ View ]

git apply --exclude=modules/system/system.tar.inc --reject 1109202-42_coding_standards.patch

Rejected files:
#       misc/ajax.js.rej
#       modules/field/field.form.inc.rej
#       modules/field_ui/field_ui.test.rej
#       modules/user/user.admin.inc.rej

I could fix all hunks manually, only ajax.js does not contain the white space issues anymore (was probably fixed elsewhere).

Are you generating these patches by making manual changes to core? Has anyone run core through the Grammar Parser recently? (It has been used previously for just this purpose.) It should clean up all the white space issues and other coding standards. It won't fix all of the comment stuff or change the JS files, though.

I use a combination of coder module, manual changes and regular expressions. I can roll a new patch one more time but it seems that this will never be committed.

We can try again but I do not know if we can attract the core commiters to commit this huge patch.

It has been impossible to commit a patch like this until recently, because we were above our issue queue thresholds. Now, I think the key is timing this properly, and announcing it ahead of time so that people know that all of their patches will break in one fell swoop.

xref: #1266446: Core-wide code style cleanup

Good job on this :)

Ok! A new updated re-rolled patch is comming! :D

StatusFileSize
new158.73 KB
FAILED: [[SimpleTest]]: [MySQL] 32,941 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

Go! ...and yes, system.tar.inc and LICENSE.txt excluded :D

Status:Needs review» Reviewed & tested by the community

I gave this a quick review. Should be commited as fast as possible.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, 1109202-55-coding-standards.patch, failed testing.

Auwch, one test expects different output. Srry oriol_e9g

StatusFileSize
new158.2 KB
PASSED: [[SimpleTest]]: [MySQL] 32,939 pass(es).
[ View ]

Again!
modules/filter/tests/filter.url-input.txt and modules/filter/tests/filter.url-output.txt excluded!

Er. Wait. I didn't mean "Re-roll it right now." I meant "We should figure out an opportune time for this to go in, and re-roll it then."

Right now we're back above our issue thresholds again, for example. :(

No problem @webchick :D We can see this like a training for a futur solid fast reroll .)

We are waiting and when you say: now! We can do the final re-roll :D

Status:Needs work» Needs review

NR for Testbot validation.

Remider: When we do the last re-roll exclude in the patch the files:

modules/system/system.tar.inc
LICENSE.txt
modules/filter/tests/filter.url-input.txt
modules/filter/tests/filter.url-output.txt

http://drupal.org/node/22336 Will be released on 1st November.
We can do our last re-roll in October 31, November 1 or the day after Novembre 2.
I'm using regular expressions and coder module to roll this patch, so we can do the last re-roll with similar effort after and before.

It's a good idea commit both patches together and only break issue queue patches one time.

I'd recommend doing it before, unless you're coordinating with the Coder module maintainers to make sure Coder looks in the /core directory.

Yes. Doing it before. #22336: Move all core Drupal files under a /core folder to improve usability and upgrades, could rollback if that has any unknown error..

is it the right time to reroll it ?

Yes, maybe we can return to this epic war.

Status:Needs review» Needs work

I'd like to help with this. @oriol_e9g, it sounds like you have a script to generate what you've done above. If you can regenerate it now that #22336: Move all core Drupal files under a /core folder to improve usability and upgrades is complete, I'll review the result with Drupal Code Sniffer to make any manual fixes necessary. (If you can explain how to apply the patch you've already submitted now that everything's in the core/ directory, I'll just work with that, but I don't know how to apply it in light of that change.)

StatusFileSize
new39.09 KB
PASSED: [[SimpleTest]]: [MySQL] 35,915 pass(es).
[ View ]

I think my intentions were a little more expansive than what's represented in the patch in #59. Attached, for example, is what I would do with aggregator module. I just worked through the output of the Drupal Code Sniffer. I think expanding the DockBlocks as it prescribes would be most helpful. Can anyone comment on my patch? Am I going in the right direction? Should I continue on to the other modules and includes?

Status:Needs work» Needs review

I commend you for taking this on. The only issue that I noticed in your patch is the addition of $form and $form_state @param directives. My understanding of the documentation standards is that these should be excluded. Once done, I will take a closer look at your patch.

Nota bene: Think about breaking this initiative up into chunks. I am not sure what the right size should be. You might want to check with Jennifer, Angie or Nat directly.

Assigned:oriol_e9g» Unassigned
StatusFileSize
new38.17 KB
PASSED: [[SimpleTest]]: [MySQL] 35,930 pass(es).
[ View ]

Thank you, @Lars Toomre. Here's an updated patch. I've filed a couple of issues for clarification of questions that have come up working on this:

My experience with this issue says that it's better to split this task in diferent issues (by module or by file).

It's too hard to commit a big patch that broken a lot of other patches in the queue.

Open a diferent issue by module and mantain here a list of issues for monitoring. I can help with patches and reviewing.

Status:Needs review» Active

Title:Drupal coding standards fixesFix coding standards violations across Drupal core

Can do. See the updated description.

Title:Fix coding standards violations across Drupal core[meta] Fix coding standards violations across Drupal core

If you're going to start a meta-issue, you should also start by listing out all the modules to be done. This is a huge effort to coordinate... See
#1310084: [meta] API documentation cleanup sprint
for an example.

Best of luck... but really IMO you would be better off working on automated tools to fix these problems rather than manual patches. That API docs cleanup has been going for months and we're getting close, but it has been a real battle...

StatusFileSize
new26.02 KB

Yes! This is big battle! Be sure you're ready for the challenge.

Status:Active» Closed (duplicate)

Issue summary:View changes

Update issue description.