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

Remove old closures compatibility code #3347

Merged
merged 1 commit into from
Oct 19, 2015

Conversation

stloyd
Copy link
Contributor

@stloyd stloyd commented Sep 24, 2015

Q A
Bug fix? no
BC breaks? no
License MIT

@stloyd
Copy link
Contributor Author

stloyd commented Oct 8, 2015

->validate(function () use ($cart) {
return !$cart->isEmpty();
->validate(function () {
return !$this->cartProvider->getCart()->isEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not $this->getCurrentCart()->isEmpty()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH. I was not sure about this, and decided to not use protected method but direct call, can change if it's a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for $this->getCurrentCart()->isEmpty() for the sake of code readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@michalmarcinkowski
Copy link
Contributor

👍

1 similar comment
@umpirsky
Copy link
Contributor

umpirsky commented Oct 8, 2015

👍

@stloyd
Copy link
Contributor Author

stloyd commented Oct 12, 2015

@pjedrzejewski ping :)

pjedrzejewski pushed a commit that referenced this pull request Oct 19, 2015
Remove old closures compatibility code
@pjedrzejewski pjedrzejewski merged commit c1bfe95 into Sylius:master Oct 19, 2015
@pjedrzejewski
Copy link
Member

Thank you Joseph!

@stloyd stloyd deleted the bugfix/clousures branch October 20, 2015 06:49
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Remove old closures compatibility code
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Remove old closures compatibility code
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Remove old closures compatibility code
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Remove old closures compatibility code
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.

None yet

4 participants