-
Notifications
You must be signed in to change notification settings - Fork 9.4k
39362: remove old code for PHP versions we no longer support #40206
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
base: 2.4-develop
Are you sure you want to change the base?
39362: remove old code for PHP versions we no longer support #40206
Conversation
Hi @SilinMykola. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
9655b41
to
d97ff1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes outdated code that was specifically written to handle compatibility with HHVM (Hip Hop Virtual Machine) and older PHP versions that are no longer supported.
- Removes HHVM-specific test skips and workarounds across numerous test files
- Updates hash algorithm default from conditional to always use 'xxh128'
- Removes PHP version checks and HHVM references from configuration files
Reviewed Changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
pub/.htaccess | Updates comment to remove HHVM reference |
lib/internal/Magento/Framework/Stdlib/DateTime/DateTimeFormatter.php | Simplifies initialization to always use IntlFormatObject |
lib/internal/Magento/Framework/Jwt/JwkFactory.php | Removes resource cleanup for older PHP versions |
app/code/Magento/CatalogImportExport/Model/Import/Product.php | Updates hash algorithm to always use xxh128 |
Multiple test files | Removes HHVM-specific test skips and compatibility checks |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
->method('execute') | ||
->willThrowException(new \PDOException('test message')); | ||
$this->setQueryStringForPdoStmtMock($query); | ||
$this->pdoStatementMock->queryString = $query; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Direct property assignment to a mock object might fail if queryString is not a defined property on the mock. Consider using the expects()
method or ensuring the mock is properly configured to accept this property.
$this->pdoStatementMock->queryString = $query; | |
$this->pdoStatementMock->expects($this->any()) | |
->method('__get') | |
->with('queryString') | |
->willReturn($query); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SilinMykola, please double-check this place, most likely the fix is needed here
Nice catch! Wanted to remove HHVM-related stuff ages ago :) |
Description (*)
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)