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

Extract parallel execution into a dedicated class #86

Merged
merged 1 commit into from
Oct 1, 2022

Conversation

theofidry
Copy link
Collaborator

A design issue with the current Parallelization trait is that:

  • it relies on the Container for which the behaviour changed
  • it provides little way to by-pass the Container which is a problem in a non Symfony App context
  • it provides private code but it is actually not protected due to being in a trait
  • a lot of verbose code has been added to accommodate the diversity of setups, which is reflected in the number of parameters passed
  • the piece of code is a nightmare to test
  • in the future we also want to provide a command instead of the trait. The trait can stay as it is handy and extending commands is sometimes awkward/impossible, but the UX could be made better via a command. However the current trait implementation does not play well with it.

I'm proposing to extract it into a dedicated class. The current implementation simply ported all the code as-is. I do plan to rework it to make easier to use.

@theofidry theofidry merged commit f4f2172 into webmozarts:master Oct 1, 2022
@theofidry theofidry deleted the feature/parallel-executor branch October 1, 2022 13: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.

1 participant