-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[FlowBundle] missing specs and CS #2863
Conversation
arnolanglade
commented
Jun 13, 2015
Q | A |
---|---|
Bug fix? | no |
New feature? | refactoring |
BC breaks? | no |
Deprecations? | no |
Fixed tickets | no |
License | MIT |
Doc PR | - |
->add('welcome', new Step\WelcomeStepAbstract()) | ||
->add('check', new Step\CheckStepAbstract()) | ||
->add('configure', new Step\ConfigureStepAbstract()) | ||
->add('setup', new Step\SetupStepAbstract()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that change was unwanted :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I will revert it !
@Arn0d and one more thing... We should avoid fluent interfaces unless there is a really good reason for that. Related article http://ocramius.github.io/blog/fluent-interfaces-are-evil/, hope it will convince you. You're doing really good job! 👍 |
Agree with @michalmarcinkowski about fluent interfaces. :) |
@michalmarcinkowski @pjedrzejewski updated ! |
ping @pjedrzejewski :p |
👍 |
ping @pjedrzejewski |
[FlowBundle] missing specs and CS
I'm not sure it was a good idea to rewrite these tests in specs? Specs have biggest benefit when they drive the design and code, otherwise they are very similar to phpunit tests, except with a nicer syntax and easier mocking/modification in future. That being said, you did it already so it is good! Thank you Arnaud! |
It is for consistency and for future development! |
This PR is a BC break since it renames Added to #3393. |