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 support for future script macros #1871

Merged
merged 1 commit into from Feb 4, 2019

Conversation

@ksolo
Copy link
Contributor

ksolo commented Feb 1, 2019

libxml2 added support for future script macros based on the below
specification

https://www.w3.org/TR/html401/appendix/notes.html#h-B.7.1

This syntax was reserved, but has yet to be defined. It is still unclear
how these macros will be used and what risks will be created if/when
implemented.

This PR removes support until more information is provided.

@flavorjones

This comment has been minimized.

Copy link
Member

flavorjones commented Feb 1, 2019

Hey @ksolo! Thanks so much for submitting this, I agree we should remove this support in Nokogiri.

It looks like this PR failed CI because the test you added is being run even when Nokogiri is built against the "system" libxml2 (in our CI, this is Ubuntu). We should skip this test unless we're running with the vendored libxml2.

An example of how to do this is in https://github.com/sparklemotion/nokogiri/blob/master/test/html/test_attributes.rb

Basically, skip the test if Nokogiri::VersionInfo.instance.libxml2? && Nokogiri::VersionInfo.instance.libxml2_using_system?

Does that make sense?

@ksolo

This comment has been minimized.

Copy link
Contributor Author

ksolo commented Feb 1, 2019

@flavorjones That makes perfect sense. Thank you!

libxml2 added support for future script macros based on the below
specification

https://www.w3.org/TR/html401/appendix/notes.html#h-B.7.1

This syntax was reserved, but has yet to be defined. It is still unclear
how these macros will be used and what risks will be created if/when
implemented.

This PR removes support until more information is provided.
@ksolo ksolo force-pushed the ksolo:revert-libxml2-patch branch from fbd54a0 to f9d58f6 Feb 1, 2019
@codeclimate

This comment has been minimized.

Copy link

codeclimate bot commented Feb 1, 2019

Code Climate has analyzed commit f9d58f6 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 93.5%.

View more on Code Climate.

@tenderlove tenderlove merged commit 86dded1 into sparklemotion:master Feb 4, 2019
4 checks passed
4 checks passed
codeclimate All good!
Details
codeclimate/diff-coverage 100% (80% threshold)
Details
codeclimate/total-coverage 93%
Details
concourse-ci/status Concourse CI build success
Details
@flavorjones

This comment has been minimized.

Copy link
Member

flavorjones commented Feb 6, 2019

👍

@flavorjones flavorjones added this to the v1.10.x patch releases milestone Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.