Includes fix from: #1893752: Make sure that redirect module does not inject redirect into discount form
A couple typos.
Code cleanup and DCS compliance.

There is still some more code cleaning to do, but this is a start.

Comments

jwjoshuawalker’s picture

attaching.

bojanz’s picture

Status: Needs review » Needs work

I'll commit #1893752: Make sure that redirect module does not inject redirect into discount form separately, let's roll a patch without that.

Most of the code wrapping changes need to be reverted.
Please see http://drupal.org/coding-standards#linelength

Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 chars.

Conditions should not be wrapped into multiple lines.

There are multiple examples of this in your patch:

+      $order->commerce_discounts[LANGUAGE_NONE][$delta]['target_id']
+        = $discount_wrapper->discount_id->value();

+  $discount_line_item_wrapper->commerce_unit_price->data
+    = commerce_price_component_add($base_price, 'base_price', $base_price, TRUE);


+  commerce_discount_add_price_component($discount_line_item_wrapper,
+    $discount_name, $discount_amount);

+  $form_state['redirect'] = 'admin/commerce/store/discounts/manage/' .
+    $form_state['commerce_discount']->name . '/delete';

We never wrap variable declarations or function calls.

jwjoshuawalker’s picture

Ok :(

I wish the codesniffer were more accurate in this regard. It does not flag an issue either way. I think that is just my personal preference, but I'll take those out where appropriate.

jwjoshuawalker’s picture

Title: Patch for UI fix, typos, and a few DCS issues » Patch for code cleanup, typos, and a few DCS issues
Status: Needs work » Needs review
StatusFileSize
new10.78 KB

Here we go.

vasike’s picture

Status: Needs review » Needs work

the (#4) patch needs a reroll.

jkuma’s picture

Thanks for the code cleanup drastik, your patch will be committed after the alpha3.

joelpittet’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new5.66 KB

Re-roll of #4

joelpittet’s picture

Status: Needs review » Fixed

Thanks, fixed and committed.

  • joelpittet committed 4013c0f on 7.x-1.x
    Issue #1985668 by drastik, joelpittet: Patch for code cleanup, typos,...

Status: Fixed » Closed (fixed)

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

The last submitted patch, 1: commerce_discount-multi-issue-patch-1985668-1.patch, failed testing.

The last submitted patch, 4: commerce_discount-multi-issue-patch-1985668-2.patch, failed testing.