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

Replaced strpos() calls with PHP8 string functions #3258

Merged
merged 4 commits into from
May 30, 2023
Merged

Replaced strpos() calls with PHP8 string functions #3258

merged 4 commits into from
May 30, 2023

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented May 14, 2023 •
edited
Loading

Description (*)

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

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Catalog Relates to Mage_Catalog Component: Cms Relates to Mage_Cms Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: Dataflow Relates to Mage_Dataflow Component: Eav Relates to Mage_Eav Component: Index Relates to Mage_Index Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: lib/Magento Relates to lib/Magento Component: Media Relates to Mage_Media Component: Oauth Relates to Mage_Oauth Component: Page Relates to Mage_Page Component: ProductAlert Relates to Mage_ProductAlert Component: Rule Relates to Mage_Rule Component: Sales Relates to Mage_Sales Component: SalesRule Relates to Mage_SalesRule Component: Widget Relates to Mage_Widget Component: Wishlist Relates to Mage_Wishlist labels May 14, 2023
@elidrissidev
Copy link
Member

Should also drop the PHP 8 polyfills to use the native functions instead?

@sreichel
Copy link
Contributor Author

sreichel commented May 23, 2023 •
edited
Loading

Maybe got you wrong, but we need the polyfills as long we support php7 (?)

@fballiano
Copy link
Contributor

we could remove the polyfills in next, but not in main ;-)

@elidrissidev
Copy link
Member

My bad I thought main was where php 7 support was dropped!

@fballiano
Copy link
Contributor

was this PR generated with rector or manually? just to know the level of detail we've to go in reviewing it.

@sreichel
Copy link
Contributor Author

Manual regex search & replace.

elidrissidev
elidrissidev approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@elidrissidev elidrissidev left a comment

Choose a reason for hiding this comment

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

Reviewed all files manually. LGTM.

fballiano
fballiano approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

also reviewed all the lines by eyes and seems everything is ok 🤞

fballiano
fballiano approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

also reviewed all the lines by eyes and seems everything is ok 🤞

@fballiano fballiano merged commit 1f54fc7 into OpenMage:main May 30, 2023
@fballiano fballiano changed the title Use php8 string functions Replaced strpos() calls with PHP8 string functions May 30, 2023
@sreichel sreichel deleted the str-php8 branch August 2, 2023 18:26
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 approved these changes

Assignees
No one assigned
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Catalog Relates to Mage_Catalog Component: Cms Relates to Mage_Cms Component: Core Relates to Mage_Core Component: Cron Relates to Mage_Cron Component: Dataflow Relates to Mage_Dataflow Component: Eav Relates to Mage_Eav Component: Index Relates to Mage_Index Component: lib/Magento Relates to lib/Magento Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Media Relates to Mage_Media Component: Oauth Relates to Mage_Oauth Component: Page Relates to Mage_Page Component: ProductAlert Relates to Mage_ProductAlert Component: Rule Relates to Mage_Rule Component: Sales Relates to Mage_Sales Component: SalesRule Relates to Mage_SalesRule Component: Widget Relates to Mage_Widget Component: Wishlist Relates to Mage_Wishlist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sreichel @elidrissidev @fballiano