Move to PHP 8.1 minimum version, add typing #97
Conversation
and add it to action
(really it's to trigger an action!)
|
Hello, I have tested the new version in my application, it works fine. While there is still some types to add here and there, I think this could already be reviewed and merged, the changeset is big enough as it is. |
|
great stuff, I need to upgrade my doodad that uses this library so I'll give it a go next week 🙏 |
|
I'm having a Christmas vacation currently; I hope to be able to spend some time on this next week. Merry Christmas, happy holidays everyone! Thank you all so much for contributing! 🎅🎄 |
There was a problem hiding this comment.
Predictably, this works for me which is nice to see. I think there are some readability improvements and consistency factors to address as I've requested changes on.
I haven't duplicated my comments but most of them apply to more than just the file I wrote them on.
I'd also like to see the screws turned up on the static analysis and linting to make it really chefs kiss. It might be worth looking at Easy Coding Standards to combine the lint and code quality part.
use expectNotToPerformAssertions() instead
|
Thanks for your comments @willpower232. I have adressed them. |
|
Hello @willpower232 and @RobThree. Shall we proceed with this? |
|
@NicolasCARPi We should First of all, thank you for your work. I've just been swamped at work and haven't had the energy to look into this a little deeper. I will do so ASAP, promised. |
|
Hello, this is a gentle ping :) |
|
I was gonna take this up this weekend. Guess what? My ESXi server's HDD decided to die on me last fridaynight 😡 So now I'm spending the weekend restoring VM's. Thank God I make regular backups etc. and I have disks ready on the shelf, but this weekend is gone. I promise to try to take a look next week. AFAIK there weren't many, if any at all, issues so it should be quite quick and easy.
|
|
Ok. It took a little longer than I had hoped but I think I'm ok with calling this a 2.0. What would you guys recommend? I was thinking (for now) to branch the current to a 1.X branch and merge this PR to master? Also @NicolasCARPi and @willpower232 : I would be honoured to put your names in the composer.json file under authors. Would you be ok with that and, if so, please provide me with the details you'd like me to put there (or make a PR with the suggested values)? |
Sounds good to me! Just pushed a commit for |
There was a problem hiding this comment.
sounds like a plan 😎
@NicolasCARPi would you mind adding my details in the same style as yours? linking to github is fine.
also CI is sad because its now using phpunit 10, could you set it to 9 or 10 instead of @stable in composer.json (and if you go with 10, you'll need to vendor/bin/phpunit --migrate-configuration)
|
@willpower232, I added you. Didn't realize the phpunit dependency was set to |
There was a problem hiding this comment.
looks great, thanks!
@RobThree version 2.0 makes sense since the composer.json requirements have changed dramatically
|
I'll make a release soon; I hate to say it (and keep saying it) but I'm a bit busy... I'll try to get around to this in a few days (though the weekend is fully booked by the misses... again... 🙄 ) |
|
You know what, that would be great. If you'd be so kind, then please, go ahead. |
As discussed in #63, here is a set of changes to bring this lib into modern PHP.
I'll leave this as a draft for now, as it's not 100% complete, but this gives you an opportunity to start the review process and leave comments.
~Nico