Skip to content

Add support for PHP 8.1 and 8.2 #147

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

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

javiereguiluz
Copy link
Collaborator

No description provided.

@weaverryan
Copy link
Contributor

Odd, there are failures on the new versions. Did you expect that?

@wouterj wouterj force-pushed the test_new_php_versions branch from 3ee8e76 to 573232e Compare January 15, 2023 13:09
@wouterj wouterj force-pushed the test_new_php_versions branch from 573232e to dcd1a74 Compare January 15, 2023 13:11
@wouterj
Copy link
Contributor

wouterj commented Jan 15, 2023

Test are now fixed. The new builds used symfony/dom-crawler 6.2, which comes with HTML5 support by default (#44170).

HTML5 parsing is enabled if the document starts with an HTML5 doctype, and our expected HTML files did not use a doctype (they are very incomplete HTML files). I've now always installed the HTML5 parser (which syncs test results between all PHP versions in CI) and added the HTML5 doctype to the expected HTML.

Copy link
Contributor

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

❤️

@javiereguiluz
Copy link
Collaborator Author

Wouter, thanks a lot for fixing this. I didn't have time to do it so I just created the failing PR ... but you did all the needed investigation and fixing 😍 Thanks!

@javiereguiluz javiereguiluz merged commit 9d4cb70 into symfony-tools:main Jan 19, 2023
@javiereguiluz javiereguiluz deleted the test_new_php_versions branch January 19, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants