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.

Sign up for GitHub

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

Jump to bottom

[API][CatalogPromotion] refactor Variants Provider to strategy Pattern #13145

Merged
merged 1 commit into from Sep 28, 2021
Merged

[API][CatalogPromotion] refactor Variants Provider to strategy Pattern #13145

merged 1 commit into from Sep 28, 2021

Conversation

AdamKasp
Copy link
Contributor

@AdamKasp AdamKasp commented Sep 27, 2021 •
edited

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

@AdamKasp AdamKasp requested a review from a team as a code owner September 27, 2021 17:44
@AdamKasp AdamKasp force-pushed the create-catalog-promotion-rule-based-on-taxons branch from 35229b2 to afbe7e4 Compare September 28, 2021 08:48
@AdamKasp AdamKasp changed the title [WIP][API][CatalogPromotion] new rule based on taxons [API][CatalogPromotion] refactor Variants Provider to strategy Pattern Sep 28, 2021
@AdamKasp AdamKasp force-pushed the create-catalog-promotion-rule-based-on-taxons branch 2 times, most recently from 4c6638b to 7f4152f Compare September 28, 2021 09:00
@Zales0123 Zales0123 added DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). labels Sep 28, 2021
@AdamKasp AdamKasp force-pushed the create-catalog-promotion-rule-based-on-taxons branch 2 times, most recently from b27e1aa to 92f6bc9 Compare September 28, 2021 12:57
Zales0123
Zales0123 approved these changes Sep 28, 2021
View reviewed changes
src/Sylius/Bundle/CoreBundle/Resources/config/services.xml
<argument type="service" id="sylius.repository.product_variant" />
<tag name="sylius.catalog_promotion.variants_provider" />
Copy link
Member

Choose a reason for hiding this comment

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

Btw, we should probably mention this tag somewhere in the documentation 🖖

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely we should, but IMHO it should be in separate PR (with an explanation of how it works with more rules), I will add subtask to our board :)

src/Sylius/Bundle/CoreBundle/spec/Provider/CatalogPromotionVariantsProviderSpec.php Outdated Show resolved Hide resolved
@AdamKasp AdamKasp force-pushed the create-catalog-promotion-rule-based-on-taxons branch from 92f6bc9 to d432c62 Compare September 28, 2021 13:44
@Zales0123 Zales0123 merged commit 15d38e9 into Sylius:master Sep 28, 2021
@Zales0123
Copy link
Member

Thanks, Adam! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GSadee GSadee GSadee left review comments

@Zales0123 Zales0123 Zales0123 approved these changes

Assignees
No one assigned
Labels
DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AdamKasp @Zales0123 @GSadee