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 null value not passed to str_replace #2901

Merged

Conversation

mattdavenport
Copy link
Contributor

Typecasing potentially default 'null' configuration values to a string before passing to str_replace, avoiding PHP 8.1 deprecation warnings.

@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Jan 5, 2023
Default configuration values may be of null value, and should be
typecasted to a string before passing to the str_replace function
@sreichel
Copy link
Contributor

sreichel commented Jan 5, 2023

Duplicate of #2806. Please keep it open for now. Waiting for review for my PR.

@sreichel sreichel added the PHP 8.1 Related to PHP 8.1 label Jan 6, 2023
@sreichel
Copy link
Contributor

sreichel commented Jan 7, 2023

Btw ... i agree with tis PR, but if it is possible a w'd avoid type casting (i know its not possible everywhere).

@mattdavenport
Copy link
Contributor Author

Agreed, I think the aforementioned PR is better here.

@elidrissidev
Copy link
Member

The other PR is good but it changes the behavior by making config values with null in the database not overriding the default value in xml, if any. See the review comment in the PR.

@fballiano
Copy link
Contributor

it seems it would make no harm to merge this PR. it's simple and shouldn't have any side effect

@sreichel sreichel reopened this Jan 9, 2023
@fballiano fballiano merged commit 901fcd1 into OpenMage:1.9.4.x Jan 9, 2023
@fballiano
Copy link
Contributor

cherry-picked to 20.0 since there were no conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core PHP 8.1 Related to PHP 8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants