Skip to content

Conversation

@sreichel
Copy link
Contributor

@sreichel sreichel commented Nov 15, 2025

Copilot AI review requested due to automatic review settings November 15, 2025 08:35
@github-actions github-actions bot added Component: Core Relates to Mage_Core Template : admin Relates to admin template Component: Sales Relates to Mage_Sales Component: Usa Relates to Mage_Usa Component: Adminhtml Relates to Mage_Adminhtml composer Relates to composer.json phpstan phpunit labels Nov 15, 2025
Copilot finished reviewing on behalf of sreichel November 15, 2025 08:37
Copy link
Contributor

Copilot AI left a 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 replaces the Zend_Measure library with the php-units-of-measure library for unit conversion functionality. This is part of modernizing dependencies and moving away from Zend Framework components.

Key Changes:

  • Adds php-units-of-measure v2.2.0 as a new dependency
  • Introduces new helper classes with measurement unit constants
  • Refactors unit conversion methods to use the new library's API

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
composer.json Adds php-units-of-measure dependency
composer.lock Includes the new library metadata
app/code/core/Mage/Core/Helper/Measure/Weight.php New helper class defining weight unit constants
app/code/core/Mage/Core/Helper/Measure/Length.php New helper class defining length unit constants
app/code/core/Mage/Usa/Helper/Data.php Refactored conversion methods to use new library API
app/code/core/Mage/Usa/Model/Shipping/Carrier/Usps.php Updated to use new measurement constants
app/code/core/Mage/Usa/Model/Shipping/Carrier/Ups.php Updated to use new measurement constants
app/code/core/Mage/Usa/Model/Shipping/Carrier/Fedex.php Updated to use new measurement constants
app/code/core/Mage/Usa/Model/Shipping/Carrier/Dhl.php Updated to use new measurement constants
app/code/core/Mage/Usa/Model/Shipping/Carrier/Dhl/International.php Updated to use new measurement constants
app/code/core/Mage/Usa/Block/Adminhtml/Dhl/Unitofmeasure.php Updated to use new measurement constants
app/design/adminhtml/default/default/template/sales/order/shipment/packaging/popup.phtml Updated template to use new measurement constants
tests/unit/Mage/Usa/Helper/DataTest.php New unit tests for conversion methods
tests/unit/Traits/DataProvider/Mage/Usa/Helper/DataTrait.php New test data provider trait
.phpunit.dist.xml Added Mage_Usa test suite
.phpstan.dist.baseline.neon Updated baseline with new exceptions

@sreichel sreichel requested review from Hanmac, addison74 and kiatng and removed request for Hanmac, addison74 and kiatng November 16, 2025 12:30
Hanmac
Hanmac previously approved these changes Nov 16, 2025
# Conflicts:
#	composer.lock
# Conflicts:
#	.phpstan.dist.baseline.neon
#	app/code/core/Mage/Usa/Helper/Data.php
# Conflicts:
#	.phpstan.dist.baseline.neon
#	app/code/core/Mage/Usa/Helper/Data.php
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 16, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
1.2% Duplication on New Code

See analysis details on SonarQube Cloud

@Hanmac
Copy link
Contributor

Hanmac commented Nov 17, 2025

Why can't phpstan find: PhpUnitsOfMeasure\PhysicalQuantity\Length? 🤔

@sreichel
Copy link
Contributor Author

sreichel commented Nov 18, 2025

What do you mean? One commit was not complete, but that should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Adminhtml Relates to Mage_Adminhtml Component: Core Relates to Mage_Core Component: Sales Relates to Mage_Sales Component: Usa Relates to Mage_Usa composer Relates to composer.json improvement phpstan phpunit Template : admin Relates to admin template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants