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

[Order] Refactor order state machine #5516

Merged
merged 2 commits into from
Jul 21, 2016

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Jul 15, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Related tickets
License MIT

Based on #5505

@bendavies
Copy link
Contributor

why has sequence/SequenceBundle been removed?
I was going to use this in a project soon. It's pretty good as a standalone.

@GSadee
Copy link
Member Author

GSadee commented Jul 15, 2016

@bendavies it is a question to #5505. Sequence and SequenceBundle are going to be removed because of its redundancy. We were using it only to generate order number in only one specific case, so this simple logic should be in OrderBundle.

@bendavies
Copy link
Contributor

That doesn't negate the fact that it's a nice standalone component. demonstrated by the fact i was going to use it.

I think it would be a shame.

@GSadee GSadee force-pushed the state-machine-order branch 4 times, most recently from bc4f71f to dc61169 Compare July 19, 2016 07:08
@GSadee GSadee changed the title [WIP][Order] Refactor order state machine [Order] Refactor order state machine Jul 19, 2016
/**
* @author Grzegorz Sadowski <grzegorz.sadowski@lakion.com>
*/
class OrderNumberGenerator implements OrderNumberGeneratorInterface
Copy link
Member

Choose a reason for hiding this comment

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

This class could be final as well

@GSadee GSadee force-pushed the state-machine-order branch 3 times, most recently from 25d964b to 7bb1265 Compare July 19, 2016 12:18
@michalmarcinkowski
Copy link
Contributor

@bendavies you can still use v0.19 of this component if you want to. We already have so many packages that we need to reduce it to minimum. We don't want to maintain this component as a generic and standalone, cause it's more overhead than benefit for us.

@pjedrzejewski pjedrzejewski merged commit fa9b2ba into Sylius:master Jul 21, 2016
@pjedrzejewski
Copy link
Member

Thanks Grzesiu!


$this->objectManager->flush();
}

/**
* @Given /^the customer confirmed (this order)$/
* @Given /^the customer fulfilled (this order)$/
Copy link
Contributor

Choose a reason for hiding this comment

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

This step will not be used.

@GSadee GSadee deleted the state-machine-order branch September 22, 2017 08:32
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