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

[Setup][Order] Fixed names of some steps in OrderContext #4943

Merged
merged 1 commit into from
May 4, 2016

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented May 4, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Related tickets
License MIT

@GSadee GSadee mentioned this pull request May 4, 2016
7 tasks
*/
public function theCustomerPlacedAnOrder(CustomerInterface $customer, $orderNumber)
public function aCustomerPlacedAnOrder(CustomerInterface $customer, $orderNumber)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we write articles in camelCased method names? customerPlacedOrder instead of aCustomerPlacedAnOrder and customerBoughtSingleProduct instead of theCustomerBoughtASingleProduct looks cleaner to me. Typing $this->custom... will show me all actions that customer may perform, without thinking if I should look at aCustom... or theCustom....

Moreover, we do not know in the method if it is a new customer or the one defined earlier, because it is the responsibility of parameters transformers to provide us customer instance in any case. We just get customer, nothing more or less.

@pjedrzejewski pjedrzejewski added the Behat Issues and PRs aimed at improving Behat usage. label May 4, 2016
@GSadee GSadee force-pushed the setup-order-context-fixes branch from 1b0e2f3 to 496489a Compare May 4, 2016 14:39
@pjedrzejewski pjedrzejewski merged commit 7cc70e2 into Sylius:master May 4, 2016
@pjedrzejewski
Copy link
Member

Thanks Grzesiu!

@GSadee GSadee deleted the setup-order-context-fixes branch September 22, 2017 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Behat Issues and PRs aimed at improving Behat usage.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants