-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Use the new json serializer which throws an error when failing #16154
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 the new json serializer which throws an error when failing #16154
Conversation
|
Hi @quisse. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
|
@magento-engcom-team give me test instance |
|
Hi @quisse. Thank you for your request.\nUnfortunately, I can only deploy instances for |
|
@umarch06 @magento-engcom-team any idea why the test instance can't be created? |
|
@magento-engcom-team give me test instance |
|
Hi @ishakhsuvarov. Thank you for your request. I'm working on Magento instance for you |
|
Hi @ishakhsuvarov, here is your Magento instance. |
|
Edited |
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.
Any idea what to do for bc with the Onepage Block?
|
& any idea why travis isn't building anything? |
|
@quisse looks like travis built your build, please review failures |
|
Finally 😄, i'll review asap. |
|
Hi @quisse, |
ihor-sviziev
left a comment
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.
Your changes looks good. Could you squash them into single commit and force push layout-config-serializer-fix branch?
0734673 to
3bedbc6
Compare
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/JsonHexTagTest.php
Outdated
Show resolved
Hide resolved
sivaschenko
left a comment
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.
Please see the review notes
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
|
I'll squash when the pr is ready for approval |
sivaschenko
left a comment
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.
Thanks for collaboration. Please see the review notes
lib/internal/Magento/Framework/Serialize/Serializer/JsonHexTag.php
Outdated
Show resolved
Hide resolved
lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/JsonHexTagTest.php
Outdated
Show resolved
Hide resolved
sivaschenko
left a comment
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.
Thanks for updates, I do not have any comments except adding some comment to empty phpdoc, please add it and squash changes for the delivery.
lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/JsonHexTagTest.php
Outdated
Show resolved
Hide resolved
9ca1c94 to
b7ba2bd
Compare
|
Hi @sivaschenko, thank you for the review. |
code formatting Backwards compatibility fix use JSON_HEX_TAG option for not breaking things use new serializer instead of json_encode use correct serializer in test declare return type use serializer updated unit testing, now uses serializer removed space declare strict type improved comment Test new serializer Test for greater than and lesser than symbol, required by JSON_HEX_TAG option split comment for readability Declare strict type on test Use di to inject cleanup curly brackets code cleanup removed empty phpdoc Fix BC
b7ba2bd to
da7f293
Compare
|
Hi @quisse, thank you for your contribution! |
|
Hi @quisse. Thank you for your contribution. |
Description
Use the new serializer which throws an error when json_encoding fails.
Fixed Issues (if relevant)
Contribution checklist