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

Introduce an input class #29

Merged
merged 11 commits into from
May 25, 2020
Merged

Introduce an input class #29

merged 11 commits into from
May 25, 2020

Conversation

theofidry
Copy link
Collaborator

@theofidry theofidry commented May 20, 2020

Update:

This PR now depends on #30. It introduces the Configuration object which takes care of the values (domain) validation and calculation. This allows to provide type-safe parameters with calculations easy to test and a friendly validation.


Original:

A constant pain with the Symfony configuration system is the complete lack of type safety. This is even more problematic in our case where we have some type juggling and calculation done.

This PR proposes to introduce an input class, ParallelizationInput which:

  • Takes care of the command configuration
  • Takes care of retrieving the input values and processing them

This allows to:

  • Completely hide the argument and options of Parallization from the user
  • Provide a better user-experience upon mistakes
  • Provide a type-safe API to consume
  • Make it easy to test all the permutations

@theofidry theofidry requested a review from webmozart May 20, 2020 13:52
theofidry added a commit that referenced this pull request May 25, 2020
Extracted from #29

This PR simplifies the `ParallelizationInput` introduced in #29 to stick to being a typesafe Symfony Input wrapper.
@theofidry theofidry merged commit 71baf57 into webmozarts:master May 25, 2020
@theofidry theofidry deleted the config branch May 25, 2020 20:01
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.

2 participants