Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

Conversation

@maxalmonte14
Copy link
Contributor

@maxalmonte14 maxalmonte14 commented Oct 26, 2018

Description (*)

This PR fixes an error detected by PHPStan

Fixed Issues (if relevant)

  1. Clean up Model/Import/Address.php #136: Clean up Model/Import/Address.php

Manual testing scenarios (*)

  1. Run ./vendor/bin/phpstan analyse app/code/Magento/CustomerImportExport/Model/Import/Address.php as described in Clean up Model/Import/Address.php #136, after this implementation the following errors shouldn't be present
------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Address.php
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  318    Access to an undefined property Magento\CustomerImportExport\Model\Import\Address::$_regionCollection.
  431    Access to an undefined property Magento\CustomerImportExport\Model\Import\Address::$_regionCollection.
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------

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)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Oct 26, 2018

CLA assistant check
All committers have signed the CLA.

/**
* Region collection instance
*
* @var string
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case I think the type should be \Magento\Directory\Model\ResourceModel\Region\Collection as the code will use the $regionColFactory->create() method if it is not set in the data array.

        $this->_regionCollection = isset(
            $data['region_collection']
        ) ? $data['region_collection'] : $regionColFactory->create();

*
* @var string
*/
protected $_regionCollection;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can also make this private here. Since this is technically a new variable.

@maxalmonte14
Copy link
Contributor Author

@dmanners thanks for the feedback, I'll apply the changes as soon as possible. ✌️

@maxalmonte14
Copy link
Contributor Author

Updated, rookie mistake in the docblock 😅

dmanners
dmanners previously approved these changes Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants