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

Use Laminas namespaced Mime Decode instead of Zend_Mime_Decode #204

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

peterjaap
Copy link

@peterjaap peterjaap commented Nov 20, 2023

@fredden
Copy link
Collaborator

fredden commented Nov 21, 2023

Thanks for this @peterjaap. I can see there is a reference to \Zend\Mail\Message in this file too. Do we need to change that to something in the Laminas namespace as well?

Also, there doesn't seem to be any mention of Zend nor Laminas in composer.json; do we need to add something (to require-dev) to cover this?

@peterjaap
Copy link
Author

@fredden well \Zend\Mail\Message is still used by Magento core itself too, so that should be fine.

It doesn't need to be added to the dependencies list, since these are part of Magento itself.

@peterjaap
Copy link
Author

@fredden hmm now that I look closer, I don't see \Zend\Mail\Message being used in the Magento core but I'm pretty sure Laminas introduced something that atuomatically uses the Laminas namespaced one. That just doesn't work with the old way of naming classes with the underscores, like Zend_Mime_Decode does.

@fredden
Copy link
Collaborator

fredden commented Dec 5, 2023

The error message has changed now, but I think that's to do with \Laminas\Mime\Decode::decodeQuotedPrintable() not actually decoding the quoted-printable string it's passed. I've reported this upstream, but a robot closed the issue immediately: laminas/laminas-mime#37.

I'll merge this in as it's clearly an improvement, even if the tests still fail.

@fredden fredden merged commit 455b34c into Ethan3600:2.x Dec 5, 2023
1 of 7 checks passed
@fredden
Copy link
Collaborator

fredden commented Dec 5, 2023

Thanks @peterjaap 👍

@peterjaap peterjaap deleted the fix-unit-test branch December 6, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants