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

Soft-deprecate AMP_Base_Sanitizer::get_body_node() #1141

Merged
merged 1 commit into from
May 11, 2018

Conversation

westonruter
Copy link
Member

There's no need to hard-deprecate this method with a warning. And it may be useful still to have this method to get the body when the html element is not desired.

See https://wordpress.org/support/topic/amp_base_sanitizerget_body/

@westonruter westonruter added this to the v0.7.1 milestone May 10, 2018
@westonruter westonruter requested a review from kienstra May 10, 2018 18:22
@westonruter westonruter force-pushed the update/soft-deprecate-get-body-node branch from 63c530b to cbc0ae5 Compare May 10, 2018 18:30
Copy link
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

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

Approved

Hi @westonruter,
Good idea to soft-deprecate get_body_node() , and it's nice how you added it to the Changelog.

@westonruter westonruter merged commit 7937f26 into develop May 11, 2018
@westonruter westonruter deleted the update/soft-deprecate-get-body-node branch May 11, 2018 00:29
@kienstra
Copy link
Contributor

Include in 0.7.1?

Hi @westonruter,
Do you think it'd be possible to include this in the 0.7.1 release? There have been a few support topics for this, and in one I accidentally said that this fix would be in the next release.

@westonruter
Copy link
Member Author

@kienstra good catch. I merged this into develop accidentally. I've cherry-picked cbc0ae5 into the 0.7 branch: c65cbf8

@kienstra
Copy link
Contributor

Thanks, @westonruter!

@kienstra
Copy link
Contributor

kienstra commented May 18, 2018

This topic might also be related to this:
https://wordpress.org/support/topic/amp-validation-errors-5/

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