There are a few cases of mis-used whitespace, poor code style, improper spelling and incorrectly wrapped comments. This patch corrects these issue and most of the issues found by coder tough love module.

CommentFileSizeAuthor
#2 1568322-coder-review.patch23.71 KBBevan
coder-review.patch27.87 KBBevan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Status: Needs review » Needs work
+++ b/workbench_moderation.admin.incundefined
@@ -156,7 +156,7 @@ function workbench_moderation_admin_states_form_submit($form, &$form_state) {
+  $views_message = t('Depending on the changes you have made it may be necessary to @reconfigure_views that leverage Workbench Moderation such as !workbench_moderation', array('@reconfigure_views' => l(t('reconfigure Views'), 'admin/structure/views'), '!workbench_moderation' => l('workbench_moderation', 'admin/structure/views/edit/workbench_moderation')));

This style of links in t() is actually incorrect. See http://drupal.org/node/322774 for explanation.

+++ b/workbench_moderation.admin.incundefined
@@ -339,7 +339,7 @@ function workbench_moderation_admin_check_role_form($form, &$form_state) {
+    drupal_set_message(t('Moderation is not enabled for any content types. Visit the @content_type_administration_page to enable moderation for one or more types.', array('@content_type_administration_page' => l(t('content type administration page'), 'admin/structure/types'))), 'warning');

Same here. Looks like this would sanitize the link HTML as well.

+++ b/workbench_moderation.admin.incundefined
@@ -391,9 +391,9 @@ function workbench_moderation_admin_check_role_form_submit($form, &$form_state)
+    drupal_set_message(t('View @node_permissions or @moderation_permissions for the @role role.', array(
+      '@node_permissions' => l(t('node permissions'), 'admin/people/permissions/' . $rid, array('fragment' => 'module-node')),
+      '@moderation_permissions' => l(t('moderation permissions'), 'admin/people/permissions/' . $rid, array('fragment' => 'module-workbench_moderation')),

Same here.

+++ b/workbench_moderation.admin.incundefined
@@ -405,7 +405,7 @@ function workbench_moderation_admin_check_role_form_submit($form, &$form_state)
+  drupal_set_message(t('You must check manually that this role has the appropriate transition permissions for your workflow. @moderation_permission_link.', array('@moderation_permissions_link' => l(t('View moderation permissions for this role'), 'admin/people/permissions/' . $rid, array('fragment' => 'module-workbench_moderation')))), 'warning');

Same here.

+++ b/workbench_moderation.moduleundefined
@@ -186,10 +184,9 @@ function workbench_moderation_menu_alter(&$items) {
@@ -298,7 +295,6 @@ function workbench_moderation_menu_local_tasks_alter(&$data, $router_item, $root

@@ -298,7 +295,6 @@ function workbench_moderation_menu_local_tasks_alter(&$data, $router_item, $root
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   The title for the tab.
  */
@@ -330,7 +326,6 @@ function workbench_moderation_edit_tab_title($node) {

@@ -330,7 +326,6 @@ function workbench_moderation_edit_tab_title($node) {
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   The title for the tab.
  */
@@ -453,7 +448,6 @@ function workbench_moderation_node_access($node, $op, $account) {

@@ -453,7 +448,6 @@ function workbench_moderation_node_access($node, $op, $account) {
  *   The operation being requested.
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   Boolean TRUE or FALSE.
  */
@@ -534,7 +528,6 @@ function _workbench_moderation_revision_access($node, $op) {

@@ -534,7 +528,6 @@ function _workbench_moderation_revision_access($node, $op) {
  *   The node being acted upon.
  * @param $state
  *   The new moderation state.
- *
  * @return
  *   Booelan TRUE or FALSE.
  */
@@ -563,7 +556,6 @@ function _workbench_moderation_moderate_access($node, $state) {

@@ -563,7 +556,6 @@ function _workbench_moderation_moderate_access($node, $state) {
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   Booelan TRUE or FALSE.
  */

According to coding standards (http://drupal.org/node/1354#functions) there should be an extra new line between @params and @return.

+++ b/workbench_moderation.moduleundefined
@@ -988,7 +981,7 @@ function workbench_moderation_moderate_node_types() {
@@ -1162,7 +1155,6 @@ function workbench_moderation_node_data($node) {

@@ -1162,7 +1155,6 @@ function workbench_moderation_node_data($node) {
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   The current node according to moderation.
  */
@@ -1186,7 +1178,6 @@ function workbench_moderation_node_current_load($node) {

@@ -1186,7 +1178,6 @@ function workbench_moderation_node_current_load($node) {
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   The node object of the live revision.
  */
@@ -1208,7 +1199,6 @@ function workbench_moderation_node_live_load($node) {

@@ -1208,7 +1199,6 @@ function workbench_moderation_node_live_load($node) {
  *
  * @param $node
  *   The node being acted upon.
- *
  * @return
  *   Boolean TRUE if this is the current revision. FALSE if not.
  */

Same here.

+++ b/workbench_moderation.rules.incundefined
@@ -216,16 +213,14 @@ function workbench_moderation_rules_condition_content_is_live_revision($node) {
+ * @return Boolean
+ *   TRUE if the revisions's current state matches the selected state.

Coding standards say we should use 'bool' and not 'Boolean'

+++ b/workbench_moderation.rules.incundefined
@@ -240,16 +235,14 @@ function workbench_moderation_rules_condition_contents_current_state($node, $mod
+ * @return Boolean

Same

Bevan’s picture

Status: Needs work » Needs review
FileSize
23.71 KB
hass’s picture

#2: 1568322-coder-review.patch queued for re-testing.

hass’s picture

Status: Needs work » Needs review

NOTE: Coder tough love does not implement the correct Drupal rules. Use coder module, please.

Status: Needs review » Needs work

The last submitted patch, 1568322-coder-review.patch, failed testing.

Status: Needs review » Needs work