Hi,

CivicActions is reviewing and upgrading multiple modules for use on client sites. Attached is a patch for problems found in relation to differences between Drupal 5 and Drupal 6 and some internationalization issues.

The main changes are:

  • In Drupal 6, menu item titles and descriptions should not be enclosed within t().
  • The $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.
  • In Drupal 6, the parameters to form_set_value() have changed (http://drupal.org/node/144132#set-value)
  • In Drupal 6, the parameters for form validation and submission functions have changed to $form, &$form_state (http://drupal.org/node/144132#process-params)

Cheers,
Stella

CommentFileSizeAuthor
nodequeue_codereview.patch7.73 KBstella

Comments

ezra-g’s picture

ezra-g commented
Status: Needs review » Fixed

Committed. Thanks for your attention here! A review with coder found some other, mostly minor issues, such as string concatenation. I'm planning to run a code review before the official Drupal 6 release.

stella’s picture

stella commented

Wow, that was fast! Thanks.

Regarding string concatenation, the Drupal 6 standard is like $string .'foo'. func(), i.e. no space between quoted terms and the dot, but there is a space between the dot and $variables, and the dot and functions, etc. In Drupal 7, this is changing so there is always a space on either side of the dot. The coder rules are going to be relaxed to allow both methods to be valid in Drupal 6, so if that's what Coder is complaining about at the moment, I wouldn't be worried about it.

Cheers,
Stella

ezra-g’s picture

ezra-g commented

Also, I found an extra call to t() around the title in the definition for 'admin/content/nodequeue/%nodequeue/view/%subqueue' so I removed it.

Thanks again!

merlinofchaos’s picture

merlinofchaos commented

Please don't ever commit anything that uses the Drupal 6 string concatenation rules. =)

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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