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

Code change you can make to align with PHP 7.4 #8

Closed
wants to merge 3 commits into from
Closed

Code change you can make to align with PHP 7.4 #8

wants to merge 3 commits into from

Conversation

dany-eudes
Copy link

These are the changes to align with PHP 7.4 that I see.

Just one test fails:
• MichaelRubel\Formatters\Tests\FormatterBindingsTest > string bindings with camel case is forbidden
Failed asserting that exception of type "Illuminate\Contracts\Container\BindingResolutionException" matches expected exception "MichaelRubel\Formatters\Exceptions\ShouldNotUseCamelCaseException". Message was: "Unresolvable dependency resolving [Parameter #0 [ $time ]] in class DateTime" at
V:\sys\laragon\www\laravel-formatters\vendor\laravel\framework\src\Illuminate\Container\Container.php:1108

@michael-rubel
Copy link
Owner

michael-rubel commented Dec 20, 2021

PHP 7.4 breaks the package bindings. The test with exception fails because it cannot find a proper binding for the formatter, and the bindings themselves aren't available since I've used PHP 8's str_contains for matching the formatter class.

You can dig deeper to fix it, or I can look into it later when I have time. I think I should add direct tests for this particular case. The case is actually covered by another test but it isn't that clear for those who're receiving these fails.

@michael-rubel
Copy link
Owner

Anyway, I don't have PHP 7.4 installed on any desktop/server, so I'm still considering if it is worth supporting it. Most people I'm working with already moving/moved to PHP 8.x since it was quite an improvement for the language.

@dany-eudes
Copy link
Author

It's possible a polyfill for PHP 7, safe to utilize with PHP 8.
But I think you're right, it's time to moving forward.
Thank you for your time.

<?php
if (!function_exists('str_contains')) {
    function str_contains (string $haystack, string $needle)
    {
        return empty($needle) || strpos($haystack, $needle) !== false;
    }
}
...

@michael-rubel
Copy link
Owner

Thank you for the pull request anyway!
I think all my packages will stay as encouragement for people to upgrade PHP to at least 8.0 if not done yet. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants