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

Added specs examples for overriding criteria and sorting parameters in route #1808

Merged
merged 1 commit into from
Aug 19, 2014

Conversation

sivolobov
Copy link
Contributor

If I do something wrong, please correct me.

@@ -251,6 +251,24 @@ function it_has_criteria_parameter(Parameters $parameters, Request $request)
$this->getCriteria()->shouldReturn($criteria);
}

function it_allows_override_criteria_parameter_in_route(Parameters $parameters, Request $request)
Copy link
Member

Choose a reason for hiding this comment

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

it_allows_to_override sounds a bit better, what do you think? :)

@pjedrzejewski
Copy link
Member

Looks good to me, please tweak the example names and we should be good to merge. Thanks a lot @sivolobov! How do you like phpspec? :)

@arnolanglade
Copy link
Contributor

@sivolobov 👍 Can you squash your commit and rebase it too?

@sivolobov
Copy link
Contributor Author

@sivolobov 👍 Can you squash your commit and rebase it too?

@Arn0d, It's done:-)

@sivolobov! How do you like phpspec? :)

@pjedrzejewski, it is very convenient tool. I like it. I think that I will start my next project with BDD approach. But I think that PHPStorm needs a plugin which will be able to catch all the methods from object which for specs are.

arnolanglade added a commit that referenced this pull request Aug 19, 2014
Added specs examples for overriding criteria and sorting parameters in route
@arnolanglade arnolanglade merged commit 1fbaa42 into Sylius:master Aug 19, 2014
@arnolanglade
Copy link
Contributor

@sivolobov Normally you should write spec before ;) thanks!

@sivolobov
Copy link
Contributor Author

I know it now. But it was not my commit with availability to override parameters. I just fixed bug that appears after #1794. I think that @foopang had to add specs.
But I finally was able to start learning TDD/BDD:-)
It was a nice first practice:-)

@arnolanglade
Copy link
Contributor

@sivolobov ✌️

@pjedrzejewski
Copy link
Member

👍

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