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

Added default return string to getParam() #2893

Merged
merged 1 commit into from
Jan 6, 2023
Merged

Added default return string to getParam() #2893

merged 1 commit into from
Jan 6, 2023

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Jan 4, 2023 •
edited
Loading

Description (*)

@elidrissidev i hope is okay to split your PR to have some progress here.

Related Pull Requests

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@sreichel sreichel added the PHP 8.1 Related to PHP 8.1 label Jan 4, 2023
@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Catalog Relates to Mage_Catalog labels Jan 4, 2023
@sreichel sreichel changed the title Added default return string (not null) to getParam() Added default return string to getParam() Jan 4, 2023
kiatng
kiatng approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

getParam() returns null by default, to change the default behavior to empty string is problematic because some code may check for null.

@fballiano fballiano merged commit d8a708d into OpenMage:1.9.4.x Jan 6, 2023
fballiano pushed a commit that referenced this pull request Jan 6, 2023
@sreichel @fballiano
Added default return string to getParam() (#2893)
88646fa
@fballiano
Copy link
Contributor

cherry-picked to v20 too since there were no conflicts

@sreichel sreichel deleted the fix/php8.1/getParam()-default branch January 6, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elidrissidev elidrissidev elidrissidev approved these changes

@fballiano fballiano fballiano left review comments

@Sdfendor Sdfendor Sdfendor approved these changes

@kiatng kiatng kiatng approved these changes

Assignees
No one assigned
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Catalog Relates to Mage_Catalog PHP 8.1 Related to PHP 8.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sreichel @fballiano @kiatng @Sdfendor @elidrissidev