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

refactor: pass checkout state as criterion to findBy method #612

Merged
merged 3 commits into from
Jan 23, 2020
Merged

refactor: pass checkout state as criterion to findBy method #612

merged 3 commits into from
Jan 23, 2020

Conversation

EmilMassey
Copy link
Contributor

No description provided.

@EmilMassey EmilMassey requested a review from a team as a code owner December 11, 2019 14:09
Copy link
Member

@mamazu mamazu left a comment

Choose a reason for hiding this comment

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

There might also be a list of orders by customer method in the order repository which would be worth considering.

@EmilMassey EmilMassey changed the title refactor: pass checkout state as criterion to findBy method [WIP] refactor: pass checkout state as criterion to findBy method Dec 11, 2019
@EmilMassey EmilMassey changed the title [WIP] refactor: pass checkout state as criterion to findBy method refactor: pass checkout state as criterion to findBy method Dec 11, 2019
@mamazu
Copy link
Member

mamazu commented Jan 22, 2020

I have rebased the branch to the current master and refactored the method in the order repository. Now it should be ready to be merged.

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 "repository-refactor" to update your local branch. Feel free to ask for assistance when you get stuck. 👍

@lchrusciel lchrusciel merged commit 760ed7c into Sylius:master Jan 23, 2020
@lchrusciel
Copy link
Member

Thank you, Emil! 🥇

@EmilMassey
Copy link
Contributor Author

Thank you for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants