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.

Sign up for GitHub

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

Jump to bottom

[8.x] Fixes for PHP 8.1 #37101

Merged
merged 9 commits into from Apr 23, 2021
Merged

[8.x] Fixes for PHP 8.1 #37101

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
65e5ea4
fix PHP 8.1 deprecations (broadcasting)
jrmajor Apr 23, 2021
1e93718
fix PHP 8.1 deprecations (cache)
jrmajor Apr 23, 2021
304bc82
remove returns after never-return method call
jrmajor Apr 23, 2021
1046255
fix PHP 8.1 deprecations (bus)
jrmajor Apr 23, 2021
493876b
fix PHP 8.1 deprecations (database)
jrmajor Apr 23, 2021
4838af5
fix PHP 8.1 deprecations (validation)
jrmajor Apr 23, 2021
b3bfa4a
use strict comparison
jrmajor Apr 23, 2021
6facd57
fix PHP 8.1 deprecations (mail)
jrmajor Apr 23, 2021
173362c
fix PHP 8.1 deprecations (routing)
jrmajor Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Illuminate/Broadcasting/Broadcasters/RedisBroadcaster.php
View file
Open in desktop
Expand Up @@ -20,16 +20,16 @@ class RedisBroadcaster extends Broadcaster
/**
* The Redis connection to use for broadcasting.
*
* @var string
* @var ?string
*/
protected $connection;
protected $connection = null;
Comment on lines -23 to +25
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the default in the constructor, which is bypassed by mocks. The same goes for $prefix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it will throw a type exception when it takes a "null" value, we can fix it by converting it to a string. I am saying this from the following point of view. It doesn't seem nice to me to define default values for parameters in the class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Briefly, isn't it a good solution to convert a "null" value to a string instead of defining it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem nice to me to define default values for parameters in the class.

It has already had the default value and it was defined in the constructor. Using mocks, however, bypasses the constructor and this property isn't set to its default. I've written that already in this comment.

Seems pretty nice to me, that's how default properties are meant to be used. It just has been unnecessary, as it was set in the constructor anyway (except for when this class was mocked, which nobody noticed).


/**
* The Redis key prefix.
*
* @var string
*/
protected $prefix;
protected $prefix = '';

/**
* Create a new broadcaster instance.
Expand Down