Coder review of contrib upgrade to 6.x (for DrupalCon Szeged)

Doug Green

on

August 24, 2008

Coder review of contrib upgrade to 6.x (for DrupalCon Szeged)

I am co-presenting coder at DrupalCon Szeged with my co-maintainer Stella who has been co-maintaining coder for about a year. See the session write-up This presentation will be similar to the one given in Boston. But Since we have less time, and since we are co-presenting (meaning even less time for me), I'm pre-releasing parts of the presentation in this blog. The key information I want people to get from this presentation are:
  1. You can run coder on patch files from the drush command line, and that we hope to start running this on d.o, either on the CVS checkin, or as a nightly check.
  2. There are still about a dozen common mistakes people make in regards to the Drupal Coding Standards. Just as I did in Boston, I will highlight those problems and give a few code examples.
  3. Now that people have started upgrading to Drupal 6.x, and creating 6.x branches, there are a couple common upgrade mistakes that peope are making. I will highlight those problems and give a few code examples.

Coder Drush Command Line

See Running Coder on patches through the command line

Top 10 STYLE problems found by Coder

Here's the Top 10 most common STYLE problems. See CODER-STYLE-SUMMARY.txt for the full list.
  1. 77919 Use an indent of 2 spaces, with no tabs
  2. 26637 There should be no trailing spaces
  3. 8626 Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
  4. 7657 curly braces { should end a line, not start one
  5. 7466 do not use mixed case (camelCase), use lower case and _
  6. 5654 string concatenation should be formatted without a space separating the operators (dot .) and a quote
  7. 5631 missing space after comma
  8. 4821 Control statements should have one space between the control keyword and opening parenthesis
  9. 4552 string concatenation should be formatted with a space separating the operators (dot .) and non-quote terms
  10. 2859 use a space between the closing parenthesis and the open bracket
  11. ... more

Top 20 UPGRADE 6.x problems found by Coder

Here's the Top 20 most common UPGRADE problems. See CODER-UPGRADE6x-SUMMARY.txt for the full list.
  1. 783 Menu item titles and descriptions should now no longer be wrapped in t() calls.
  2. 603 use $form_state['values'] instead, which is where the values are stored after a submit
  3. 248 Use the new Schema API to create and drop tables rather than db_query()
  4. 138 The menu system has been completely over-hauled in 6.x.
  5. 116 A new schema API has been added in 6.x
  6. 107 The arguments to hook_help() have changed
  7. 101 The parameters for form validation and submission functions have changed to $form, &$form_state.
  8. 77 New syntax for .info files files requires core=6.x
  9. 67 Submit handlers use $form_state rather than returning urls
  10. 62 When labelling buttons, make it clear what the button does, "Submit" is too generic.
  11. 60 Translations moved from ./po to ./translations
  12. 50 hook no longer exists, use hook_form_alter() to swap your own validation handler
  13. 47 Parameters of watchdog() changed, you must remove calls to t()
  14. 40 replace $form['op'] with $form_state['values']['op']
  15. 38 Use menu_get_object() to get an object based on your path
  16. 38 New syntax for .info files, use dependencies[]
  17. 37 url() and l() arguments changed, if you have a lot of these use this conversion script
  18. 31 new hook_theme() function is required to register theme_ functions
  19. 31 hook_form_alter() parameters have changed
  20. 27 Parameters to hook_access() have changed
  21. ... more

My analysis

This is how the above lists were created.
  1. Install Drupal 6.x:
    $ /cd /var/www
    $ cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal  checkout -d drupal-6 -r DRUPAL-6-4 drupal
  2. Checkout all of contrib on the DRUPAL-6--1 tag:
    $ cd drupal-6/sites/all
    $ cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout -d modules -r DRUPAL-6--1 contributions/modules
  3. Enable drush and coder on http://example.com/admin/build/modules.
  4. Copy example.drushrc.php to your sites directory:
    $ cp modules/drush/example.drush.php drush.php
  5. In drush.php set options['l'] and $options['r'] as needed. Mine are set to:
    $options['l'] = 'http://localhost/drupal-6';
    $options['r'] = '/var/www/drupal-6';
  6. Increase your php memory_limit in /etc/php5/cli/php.ini. I set mine to 1024 (which is 1G), I'm not really sure how much you need, but I do know that 32M was not enough.
  7. Run coder's reviews through drush
    $ ./drush.php coder style minor all > CODER-STYLE.txt
    $ ./drush.php coder upgrade6x minor all > CODER-UPGRADE6x.txt
  8. Then some shell magic, to summarize the warnings, in order of occurence:
    $ grep '^ +' CODER-STYLE.txt | sed -e 's/^ +[0-9]*: //' | awk '{ s[$0] ++; } END { for (l in s) { print s[l] " " l; } }' | sort -n -r > CODER-STYLE-SUMMARY.txt
    $ grep '^ +' CODER-UPGRADE6x.txt | sed -e 's/^ +[0-9]*: //' | awk '{ s[$0] ++; } END { for (l in s) { print s[l] " " l; } }' | sort -n -r > CODER-UPGRADE6x-SUMMARY.txt
AttachmentSize
CODER-STYLE.txt9.01 MB
CODER-STYLE-SUMMARY.txt1.51 KB
CODER-UPGRADE6x.txt622.8 KB
CODER-UPGRADE6x-SUMMARY.txt4.82 KB

Share it!

Insert this into the Top 20 Upgrade Mistakes. I wrote this rule after attending the DrupalCon Menu session by Peter and Chx. 8a. 76 - The menu system has been completely over-hauled in 6.x, and if you are looping, you are probably doing something wrong.
Coder does an excellent job of identifying the issues. The Deadwood module goes the next step by automatically making many of the changes needed to port a module from 5.x to 6.x. Of the top 20 items you cite, Deadwood will address all but item 15. Help spread the word about Deadwood and encourage the community to take advantage of it.
Any text editor worth its salt can take care of the first two style problems for you. I use jEdit, and its Whitespace plugin is great for dealing that kind of thing, so I don't have to. ----- Ubercart -- One cart to rule them all.