-
Notifications
You must be signed in to change notification settings - Fork 18
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
Feat: PHP8 / Symfony 5+ support #47
Conversation
Wow, awesome! Thanks for putting this together. Will review this ASAP. |
Looks like Travis isn't triggered. Do you want to keep it or do you want me to switch this repo to Github Actions? |
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.
I had a first change to look at the code. Over all LGTM (see comments, questions, requests). Again thanks for the time you spend updating all classes and especially the tests. 👍
Next (probably in the evening) I will look at the Travis setup. Looks like there is some configuration missing or which at least needs to be fixed. Will keep you posted.
Replaced Travis CI setup with Github Actions. Let me know what you think. |
Hi @lennerd, Sorry, still didn't have the time to review your review, but I don't forget it, it's still in my todos. Thanks! |
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.
Done, finally :-)
Rebased + updated Github Actions config. |
@bpolaszek not sure how Github send notifications about failed checked. So in case you didn't notice … 😬 😉 |
Hey @lennerd - sorry I keep snoozing that one 😞 Thanks! |
No problem and thanks for letting me know. Will notify you next month if needed. 😉 |
Hey @lennerd - I just took a quick look again and CI fails because Symfony's config component So, either we have to maintain 2 distinct implementations of WDYT? |
98edd4d
to
95f51e3
Compare
95f51e3
to
baf02d7
Compare
I dropped PHP < 7.4 support as well as Symfony < 5. |
Ping @lennerd :-) |
@bpolaszek sorry for the long delay! And thanks again for all the changes. Everything looks good so far. My problem is I haven‘t used this library nor Symfony or even PHP for a very long time. So to be honest maintaing this project is not my top priority anymore. But, I might have a wild idea. What do you think about beeing a contributer to this project? As a first act you could merge this PR. 🙂 |
Hi @lennerd, No worries, I understand 🙂 |
@bpolaszek Nice. 👍 Invitation should be on it‘s way. Let me know if this works for you. |
@lennerd sounds good 🙂 may I push the button? I usually use the "squash/merge" option in github in my other projects, but if you prefer merge commits, please let me know. Thanks again! 🙏 |
You may! 🙂 In the meantime I also prefer squash and merge. So I would say, go for it! |
Here we go! 🎉 |
This PR adds support for PHP8 and Symfony 5+, by bumping minimal PHP version to
7.1
and adding return types.Hence it also drops support for older Symfony versions, and will require a new major release as some BC breaks are expected.