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

Removed default value from $or variable on applyFilter function #61

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ch00n
Copy link

@ch00n ch00n commented Jan 12, 2021

Leaving the $join variable blank when using PHP 8 causes an exception when running artisan.

An empty array has been assigned to the variable to prevent this from happening.

UPDATE: Related to this deprecated PHP feature: https://stackoverflow.com/a/65297280/518704

To ensure compatibility with PHP 8, the default value for $or has been removed.

@ch00n ch00n marked this pull request as draft January 18, 2021 13:03
@ch00n ch00n marked this pull request as ready for review January 18, 2021 13:03
@gruz
Copy link

gruz commented Aug 21, 2021

I think the more correct way should be to remove $or default false
Passing The &$join must be passed as it's passed by reference. So no default value has sense here.

What concerns $or = false, it's ok to remove = false

https://stackoverflow.com/a/65297280/518704

This is a better fix for this PHP 8 deprecated feature: https://stackoverflow.com/a/65297280/518704
As @gruz  suggests, `$or` does not need a default value as  `filter_var($or, FILTER_VALIDATE_BOOLEAN)` resolves to false by default on line 130.
@ch00n
Copy link
Author

ch00n commented Sep 8, 2021

I think the more correct way should be to remove $or default false
Passing The &$join must be passed as it's passed by reference. So no default value has sense here.

What concerns $or = false, it's ok to remove = false

https://stackoverflow.com/a/65297280/518704

Thanks for pointing that out @gruz. You absolutely right, the default should be removed from $or The PR has been updated. :)

@ch00n ch00n changed the title Added default value to $join variable on applyFilter function Removed default value from $or variable on applyFilter function Sep 8, 2021
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.

3 participants