Skip to content

Conversation

@mhauri
Copy link

@mhauri mhauri commented May 17, 2018

Description

Method name \Magento\Catalog\Model\Product\Option\Type::_getChargableOptionPrice contained typo.

Renamed it to _getChargeableOptionPrice.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@miguelbalparda miguelbalparda self-assigned this May 17, 2018
@magento-engcom-team
Copy link
Contributor

Hi @miguelbalparda, thank you for the review.
ENGCOM-1580 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@mhauri thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@orlangur orlangur self-assigned this May 17, 2018
* @return float
*/
protected function _getChargableOptionPrice($price, $isPercent, $basePrice)
protected function _getChargeableOptionPrice($price, $isPercent, $basePrice)
Copy link
Contributor

Choose a reason for hiding this comment

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

Protected methods cannot be removed, please check https://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html#php:

Public and protected method removal
Mark the method with the @deprecated tag instead of removing it.

Copy link
Contributor

@miguelbalparda miguelbalparda left a comment

Choose a reason for hiding this comment

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

@mhauri
Copy link
Author

mhauri commented May 18, 2018

@orlangur I've added the deprecated info in commit: 8ea661c

@magento-engcom-team
Copy link
Contributor

Hi @mhauri. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.5 release.

Please, consider porting this solution across release lines.
You may use Porting tool to port commits automatically.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants