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

Inject dependencies into FlowBundle\ProcessController. #1459

Merged
merged 1 commit into from
May 9, 2014
Merged

Inject dependencies into FlowBundle\ProcessController. #1459

merged 1 commit into from
May 9, 2014

Conversation

realityking
Copy link
Contributor

I'd like to refactor FlowBundle into a Component + Bundle as this would be useful for Silex, Drupal and projects using Symfony Components but not the whole thing.

This is a very small first step into that direction.

@pjedrzejewski
Copy link
Member

I think this is a good idea, at first, I thought FlowBundle is too specific to extract a component, but I believe we can separate the Request & Session, the rest of architecture should work just fine!

@realityking
Copy link
Contributor Author

Test on PHP 5.3 times out, but no errors occurred.

{
private $processCoordinator;
private $processContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you define them as protected and add docblocks? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

winzou added a commit that referenced this pull request May 9, 2014
Inject dependencies into FlowBundle\ProcessController.
@winzou winzou merged commit cc64877 into Sylius:master May 9, 2014
@winzou
Copy link
Contributor

winzou commented May 9, 2014

Nice step forward, thanks!

@realityking realityking deleted the flow branch May 9, 2014 07:52
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.

3 participants