Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Php 84 support #330

Merged
merged 6 commits into from
Jan 21, 2025
Merged

Php 84 support #330

merged 6 commits into from
Jan 21, 2025

Conversation

markwalet
Copy link
Contributor

This pull request attempts to upgrade laravel-zero/framework v11.

To do this, support for PHP 8.2 had to be dropped. (source)

The original reason behind this upgrade is because of deprecation warnings in the nesbot/carbon dependency while running on PHP 8.4:

bash-5.1# ./vendor/bin/pint --test
PHP Deprecated:  Carbon\Traits\Date::getDaysFromStartOfWeek(): Implicitly marking parameter $weekStartsAt as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1394
PHP Deprecated:  Carbon\Traits\Date::setDaysFromStartOfWeek(): Implicitly marking parameter $weekStartsAt as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1412
PHP Deprecated:  Carbon\Traits\Date::utcOffset(): Implicitly marking parameter $minuteOffset as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/Traits/Date.php on line 1481
PHP Deprecated:  Carbon\Traits\Localization::locale(): Implicitly marking parameter $locale as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/Traits/Localization.php on line 447
PHP Deprecated:  Carbon\Traits\Test::setDefaultTimezone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/Traits/Test.php on line 203
PHP Deprecated:  Carbon\CarbonTimeZone::toOffsetName(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 158
PHP Deprecated:  Carbon\CarbonTimeZone::toOffsetTimeZone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 172
PHP Deprecated:  Carbon\CarbonTimeZone::toRegionName(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 188
PHP Deprecated:  Carbon\CarbonTimeZone::toRegionTimeZone(): Implicitly marking parameter $date as nullable is deprecated, the explicit nullable type must be used instead in phar:///var/www/vendor/laravel/pint/builds/pint/vendor/nesbot/carbon/src/Carbon/CarbonTimeZone.php on line 230

  ........................................................................................................................................................................................
  ........................................................................................................................................................................................
  ........................................................................................................................................................................................
  ........................................................................................................................................................................................
  ........................................................................................................................................................................................
  ........................

  ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel
    PASS   ..................................................................................................................................................................... 944 files

This deprecation warnings are resolved in version 3 in Carbon, but this version is only supported in Laravel 11. So that's why I thought this bit of system maintenance could be helpful. It does drop support for PHP 8.1 but that version hasn't been in active support for over a year.

Let me know if anything else is needed. Keep up the great work! 🚀

@taylorotwell taylorotwell marked this pull request as draft January 6, 2025 15:05
@markwalet
Copy link
Contributor Author

I have no clue why phpstan is failing. There is no GitPathsRepository::processFileNames() in my repository.

@markwalet markwalet marked this pull request as ready for review January 7, 2025 08:55
@Jubeki
Copy link
Contributor

Jubeki commented Jan 7, 2025

I have no clue why phpstan is failing. There is no GitPathsRepository::processFileNames() in my repository.

You need to change the following Line:

* @param \Illuminate\Support\Collection<int, string> $fileNames

to

@param  \Illuminate\Support\Collection<int, non-falsy-string>  $fileNames

@markwalet
Copy link
Contributor Author

You need to change the following Line:

I see, it seems like it did some weird stuff because I didn't have this line in my repository. It looks like it was added recently. I synced the fork and merged main into my branch. It is changed now and the tests are successful.

@taylorotwell taylorotwell marked this pull request as draft January 7, 2025 23:24
@markwalet
Copy link
Contributor Author

@taylorotwell I see you marked the PR as draft, but the CI issues were already fixed. Any feedback I need to process before we can review again?

@Jubeki
Copy link
Contributor

Jubeki commented Jan 17, 2025

@taylorotwell I see you marked the PR as draft, but the CI issues were already fixed. Any feedback I need to process before we can review again?

I think he is waiting for a review from @nunomaduro (See the reviewers section on the right side)

@markwalet markwalet marked this pull request as ready for review January 21, 2025 10:59
@taylorotwell taylorotwell merged commit 0c31559 into laravel:main Jan 21, 2025
6 checks passed
@markwalet markwalet deleted the php-84-support branch January 21, 2025 15:01
@parallels999
Copy link

@nunomaduro hi, could you release it?? thanks

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

Successfully merging this pull request may close these issues.

4 participants