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] Use serializer instead of custom view for shipping methods #12713

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Jun 17, 2021

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets replaces #12392
License MIT

@GSadee GSadee added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). API APIs related issues and PRs. labels Jun 17, 2021
@GSadee GSadee requested a review from a team as a code owner June 17, 2021 09:52
@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label Jun 17, 2021
Copy link
Member

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

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

A lot of code removed is always nice to see 🚀

@GSadee GSadee force-pushed the api-use-serializer-instead-of-custom-view-for-shipping-methods branch 2 times, most recently from 6a13d90 to 9df6d86 Compare June 21, 2021 05:04
@GSadee GSadee force-pushed the api-use-serializer-instead-of-custom-view-for-shipping-methods branch from 4e13ab0 to e914795 Compare June 21, 2021 11:24
@Zales0123 Zales0123 merged commit ec91706 into Sylius:master Jun 21, 2021
@Zales0123
Copy link
Member

Thank you, Grzegorz! 🎉

@GSadee GSadee deleted the api-use-serializer-instead-of-custom-view-for-shipping-methods branch June 22, 2021 04:28
AdamKasp added a commit that referenced this pull request Jun 22, 2021
…shipping methods endpoint (GSadee)

This PR was merged into the 1.11-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | based on #12713 
| License         | MIT


Commits
-------

175ca05 [API] Change names of API contract tests
dedad00 [API][ShippingMethod] Add contract test for available shipping methods endpoint
/** @experimental */
final class ShippingMethodNormalizer implements ContextAwareNormalizerInterface, NormalizerAwareInterface
{
use NormalizerAwareTrait;
Copy link
Member

Choose a reason for hiding this comment

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

IIRC this trait makes it harder to decorate service in the end app. Do you think we could use regular DI for normalization logic?

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. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants