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

Remove zend json checkout #8481

Merged
merged 9 commits into from
Feb 19, 2017

Conversation

dmanners
Copy link
Contributor

@dmanners dmanners commented Feb 9, 2017

As Zend1 has been EOL we should consider removing Zend1 components in Magento2. In this pull request I am replacing the usage of Zend_Json with the Magento Framework lib's serializer json object.

*/
public function __construct(
\Magento\Framework\View\Element\Template\Context $context,
\Magento\Customer\Model\Session $customerSession,
\Magento\Checkout\Model\Session $checkoutSession,
\Magento\Checkout\Model\CompositeConfigProvider $configProvider,
array $layoutProcessors = [],
\Magento\Framework\Serialize\Serializer\Json $serializer = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you are adding a new optional param like this it should come last as you are trying to avoid breaking anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah thanks @fooman for catching that.

@okorshenko okorshenko self-assigned this Feb 10, 2017
@okorshenko okorshenko added this to the February 2017 milestone Feb 11, 2017
@mmansoor-magento mmansoor-magento merged commit 5c57dc2 into magento:develop Feb 19, 2017
@okorshenko
Copy link
Contributor

@dmanners thank you for your contribution!

@dmanners dmanners deleted the remove-zend-json-checkout branch July 26, 2017 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants