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

[Api] Receiving percentage discount #12083

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

Tomanhez
Copy link
Contributor

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

This PR is based on #12081

@Tomanhez Tomanhez added Feature New feature proposals. API APIs related issues and PRs. labels Nov 24, 2020
@Tomanhez Tomanhez requested a review from a team as a code owner November 24, 2020 19:45
@@ -541,4 +565,22 @@ private function checkProductQuantity(
}
}
}

private function compareItemSubtotal(ProductInterface $product, \Closure $price): void
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that very simillar or even the same function is here https://github.com/Sylius/Sylius/pull/12081/files/a2cef239265f11c566a0efbff8514ef2142685d4#diff-b5c163d690bfe47026cba0b16b71bdc7a4e1b0d95352b10522d4b0194d23e85dR555
Maybe we can think about another class that can have same function? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same function because this PR is based on #12081

@Tomanhez Tomanhez force-pushed the reciving-percentage-discount branch 2 times, most recently from 1d6d7e1 to dcbfc00 Compare November 26, 2020 09:56
Copy link
Member

@GSadee GSadee left a comment

Choose a reason for hiding this comment

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

Could you rebase with master?

@Tomanhez Tomanhez force-pushed the reciving-percentage-discount branch from dcbfc00 to 25afe06 Compare November 26, 2020 11:03
@GSadee GSadee merged commit c603ad1 into Sylius:master Nov 26, 2020
@GSadee
Copy link
Member

GSadee commented Nov 26, 2020

Thanks, Tomasz! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs. Feature New feature proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants