Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Order recalculation clean-up #4551

Merged
merged 6 commits into from
Mar 24, 2016

Conversation

Zales0123
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT
Doc PR
  • move all recalculation logic to OrderRecalculator
  • use OrderRecalculator as Order state machine callbacks
  • remove unnecessary "started" state and "start" transition on Order
  • recalculate order on user login

}

function it_uses_order_recalculator_to_recalculate_order(
$orderRecalculator,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this spec you have used Typehint for properties given in constructor, but here you haven't use this approach. Can you unified them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@Zales0123 Zales0123 force-pushed the order-recalculation-clean-ups branch 4 times, most recently from 8b1d59f to 0372c52 Compare March 23, 2016 09:38
/**
* @param InteractiveLoginEvent $event
*/
public function recalculateCartWhileInteractiveLogin(InteractiveLoginEvent $event)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not using $event at all we can typehint to Symfony\Component\EventDispatcher\Event class and have one method for both events ;)

@Zales0123 Zales0123 force-pushed the order-recalculation-clean-ups branch from 58f02a0 to a805e28 Compare March 24, 2016 07:04
@lchrusciel
Copy link
Member

👍

@Zales0123 Zales0123 force-pushed the order-recalculation-clean-ups branch from a805e28 to 0e4a3d5 Compare March 24, 2016 08:09
@Arminek
Copy link
Contributor

Arminek commented Mar 24, 2016

👍

@pjedrzejewski pjedrzejewski merged commit b7da969 into Sylius:master Mar 24, 2016
@pjedrzejewski
Copy link
Member

Nice work Mateusz! 👍

@Zales0123 Zales0123 deleted the order-recalculation-clean-ups branch October 28, 2016 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants