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

Ensure tests pass without warnings in PHP 8.2 #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elcapo
Copy link

@elcapo elcapo commented May 4, 2024

Hi @ronanguilloux,

I hope I'm not being too invasive with this PR. If so, feel free to cancel it.

What I'm doing is making sure things work without issues in PHP 8.2 (as tests were throwing a hundreds of warnings). Most of them were due to passing null to PHP native functions that don't support null anymore. To fix those issues, I systematically added statements in the form of:

$value = $value ?? '';

The full summary of what this PR does, is:

  • fix many calls to PHP functions where null was being passed although it's deprecated
  • update reference to laravel-isocodes-validation as the readme of the previously referenced package redirects to the one I'm pointing now
  • declare support for php 8.2 in the readme
  • fix call to undefined method PhpCsFixer\Config::create() which was necessary to be able to run make quality
  • fix psr-2 coding standard issues to fix issues found by make quality (although they were not introduced by the PR)

fix psr-2 coding standard issues

declare support for php 8.2

update reference to laravel-isocodes-validation

fix call to undefined method PhpCsFixer\Config::create()
@stof
Copy link
Contributor

stof commented Jul 10, 2024

As the phpdoc type for those parameters says @param string, the proper fix should be done in the caller IMO, by avoiding the usage of parameter values that don't respect the document contract of the class.

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.

2 participants