Make toolbar module pass Coder Review

Completed.

See #1533202-4: Make config module pass Coder Review. :)

Summarized, sorry about that.

Confirmed that all changes are style only and are correct coding standards.

I'm sorry. Per the initiative, adding param/return datatypes will be handled in a separate issue, so those changes need to be removed from the patch. Additionally, there are still style issues to fix:

FILE: /home/quickstart/websites/d8.dev/core/modules/toolbar/toolbar-rtl.css
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 22 | ERROR | Multiple CSS properties should be listed in alphabetical order
--------------------------------------------------------------------------------


FILE: /home/quickstart/websites/d8.dev/core/modules/toolbar/toolbar.css
--------------------------------------------------------------------------------
FOUND 6 ERROR(S) AFFECTING 5 LINE(S)
--------------------------------------------------------------------------------
  45 | ERROR | Each style definition must be on a line by itself
  45 | ERROR | Expected 1 space after colon in style definition; 0 found
 100 | ERROR | Expected 1 space after colon in style definition; 2 found
 103 | ERROR | Expected 1 space after colon in style definition; 2 found
 107 | ERROR | Expected 1 space after colon in style definition; 2 found
 133 | ERROR | Multiple CSS properties should be listed in alphabetical order
--------------------------------------------------------------------------------

Be sure to be using the latest copy of the code sniffer, by the way. It moves fast. I always update mine via Git before running it.

Param/return datatypes removed. Additional coder issues addressed except for the first issue in toolbar-rtl.css. It looks correct to me, so possibly a false positive?

Summarizing work done

Issue status updated with latest drupalcs report.

Thank you, @Albert Volkman, you're right; both errors are false positives. Would you like to create issues for them and add links to them in the summary here?

Sure thing-

http://drupal.org/node/1623442

I thought we weren't in agreement on the coding standard yet, regarding this change:

-  $admin_link = db_query('SELECT * FROM {menu_links} WHERE menu_name = :menu_name AND module = :module AND link_path = :path', array(':menu_name' => 'management', ':module' => 'system', ':path' => 'admin'))->fetchAssoc();
+  $args = array(
+    ':menu_name' => 'management',
+    ':module' => 'system',
+    ':path' => 'admin',
+  );
+  $admin_link = db_query('SELECT * FROM {menu_links} WHERE menu_name = :menu_name AND module = :module AND link_path = :path', $args)->fetchAssoc();

I'm also confused about these changes (two like this) -- why are they necessary/desirable?

+++ b/core/modules/toolbar/toolbar-rtl.css
@@ -18,8 +18,8 @@
   float: left;
 }
 #toolbar ul#toolbar-user li {
-  float: none;
   display: inline;
+  float: none;

Oops. I'm sorry, @jhodgdon: that first one got by me.

As to the second one, drupalcs complains about CSS properties that aren't in alphabetical order.

Alphabetical CSS -- OK -- I see that on the CSS coding standards page: http://drupal.org/node/302199

That's why I haven't rolled any new patches recently, I thought this whole thing was postponed. Kind of frustrating, really, I do want to contribute.

Updated issue status.

I think there's agreement on the coding standards now https://www.drupal.org/node/1886770

Thanks 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.

Closing in favor of #2571965: [meta] Fix PHP coding standards in core, stage 1. In this issue the coding standards will be fixed on a sniff-per-sniff basis rather than a module-per-module basis.