-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Shipping] Removed unneeded composite resolver interface #5259
[Shipping] Removed unneeded composite resolver interface #5259
Conversation
|
||
/** | ||
* @author Arnaud Langlade <arn0d.dev@gamil.com> | ||
*/ | ||
class ShippingMethodChoiceTypeSpec extends ObjectBehavior | ||
{ | ||
function let( | ||
CompositeMethodsResolverInterface $compositeMethodsResolver, | ||
MethodsResolverInterface $compositeMethodsResolver, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is called $compositeMethodsResolver
but the interface is now MethodsResolverInterface
. It's just a small thing but this now makes it seem like a composite methods resolver is expected to be injected in, even though that won't necessarily be true.
Similar occurrences elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
@CarwynNelson Thanks for the review! :) |
* @param ServiceRegistryInterface $calculators | ||
* @param RepositoryInterface $repository | ||
*/ | ||
public function __construct( | ||
CompositeMethodsResolverInterface $compositeShippingMethodsResolver, | ||
MethodsResolverInterface $compositeShippingMethodsResolver, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we should also rename the variable.
53b89e7
to
39297e2
Compare
Thank you Mateusz. :) |
Change proposed in this comment.