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.

Sign up for GitHub

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

Jump to bottom

PHP 8.0 Support #9

Merged
merged 1 commit into from
Nov 17, 2020
Merged

PHP 8.0 Support #9

merged 1 commit into from
Nov 17, 2020

Conversation

Gounlaf
Copy link
Contributor

@Gounlaf Gounlaf commented Oct 4, 2020 •
edited
Loading

This PR should check all tasks in #8

Regards

@Gounlaf Gounlaf marked this pull request as draft October 5, 2020 18:24
@Gounlaf
Copy link
Contributor Author

Gounlaf commented Oct 5, 2020

Have to rollback typehinting (BC Break)

andypost
andypost approved these changes Oct 16, 2020
View reviewed changes
Copy link

@andypost andypost left a comment

Choose a reason for hiding this comment

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

Looks great and pass tests

@froschdesign froschdesign linked an issue Oct 16, 2020 that may be closed by this pull request
PHP 8.0 support #8
Closed
6 tasks
@froschdesign froschdesign added Enhancement hacktoberfest-accepted Issues/Pull-Requests which can be fixed during Hacktoberfest: https://hacktoberfest.digitalocean.com labels Oct 16, 2020
@froschdesign froschdesign added this to the 2.7.0 milestone Oct 16, 2020
froschdesign
froschdesign requested changes Oct 16, 2020
View reviewed changes
Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

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

Have to rollback typehinting (BC Break)

A rollback is correct here.

@Gounlaf
Copy link
Contributor Author

Gounlaf commented Oct 16, 2020 •
edited
Loading

Have to rollback typehinting (BC Break)

A rollback is correct here.

It was a reminder. I pushed another commit after that to do the rollback as required 🙂

@froschdesign
Copy link
Member

@Gounlaf
Take your time and many thanks for your help! 👍 👍 👍

@Gounlaf Gounlaf marked this pull request as ready for review October 16, 2020 11:12
@Gounlaf
Copy link
Contributor Author

Gounlaf commented Oct 16, 2020

I've reverted changes related to typehinting, and squash.

boesing
boesing approved these changes Oct 16, 2020
View reviewed changes
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM.
I'd prefer having the vendor/phpunit/phpunit/phpunit.xsd instead for the schema ref but imho thats not a showstopper.

@Gounlaf
Copy link
Contributor Author

Gounlaf commented Oct 18, 2020

LGTM.
I'd prefer having the vendor/phpunit/phpunit/phpunit.xsd instead for the schema ref but imho thats not a showstopper.

I can change it if you want

@andypost
Copy link

andypost commented Nov 1, 2020 •
edited
Loading

Is there release schedule for PHP 8 support?
Drupal 9.1 (upcoming release) has it as dev-dependency and it's a blocker to declare PHP 8 support

froschdesign
froschdesign requested changes Nov 1, 2020
View reviewed changes
phpunit.xml.dist Outdated Show resolved Hide resolved
test/EscaperTest.php Outdated Show resolved Hide resolved
test/EscaperTest.php Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@Gounlaf
Copy link
Contributor Author

Gounlaf commented Nov 1, 2020

@froschdesign I have commited changes according to your review.
I will squash once it's OK

@froschdesign
Copy link
Member

@Gounlaf
You are fast! 😃 👍

@Gounlaf
Copy link
Contributor Author

Gounlaf commented Nov 1, 2020

@Gounlaf
You are fast! 😃 👍

:)

I can squash so?

@Gounlaf
Fix issue #8
598490e 
Signed-off-by: Levis Florian <[email protected]>
andypost
andypost approved these changes Nov 1, 2020
View reviewed changes
Copy link

@andypost andypost left a comment

Choose a reason for hiding this comment

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

Looks great 👍

@boesing boesing changed the title Fix issue #8 PHP 8.0 Support Nov 4, 2020
@boesing boesing merged commit 598490e into laminas:2.7.x Nov 17, 2020
@boesing
Copy link
Member

boesing commented Nov 17, 2020

Thanks, @Gounlaf!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boesing boesing boesing approved these changes

@froschdesign froschdesign froschdesign approved these changes

@andypost andypost andypost approved these changes

Assignees
No one assigned
Labels
Enhancement hacktoberfest-accepted Issues/Pull-Requests which can be fixed during Hacktoberfest: https://hacktoberfest.digitalocean.com
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.

PHP 8.0 support
4 participants
@Gounlaf @froschdesign @andypost @boesing