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 flush() call, its done in the remover itself #10147

Merged

Conversation

stefandoorn
Copy link
Contributor

Q A
Branch? 1.2
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
License MIT

Looking through the code, in the remover itself (ExpiredCartsRemover) a flush call is made already on the Order Manager. Seems double and not needed to me here.

@stefandoorn stefandoorn requested a review from a team as a code owner February 4, 2019 08:54
@pamil pamil added the Potential Bug Potential bugs or bugfixes, that needs to be reproduced. label Feb 5, 2019
@pamil pamil changed the base branch from 1.2 to 1.3 February 5, 2019 12:20
@pamil
Copy link
Contributor

pamil commented Feb 5, 2019

The base of this pull-request was changed, you need fetch and reset your local branch
if you want to add new commits to this pull request. Reset before you pull, else commits
may become messed-up.

Unless you added new commits (to this branch) locally that you did not push yet,
execute git fetch origin && git reset "order-manager-flush-remove-carts" to update your local branch.

Feel free to ask for assistance when you get stuck 👍

@pamil pamil force-pushed the order-manager-flush-remove-carts branch from 43da645 to 06115ab Compare February 5, 2019 12:20
@lchrusciel lchrusciel merged commit 9fc00a0 into Sylius:1.3 Feb 6, 2019
@lchrusciel
Copy link
Member

Thank you, Stefan! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Potential Bug Potential bugs or bugfixes, that needs to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants