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.

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

Edited doc block of the walk method in a Collection #13373

Conversation

ByteCreation
Copy link
Contributor

Description

Edited doc block of the walk method in a Collection to reflect that this method will accept an array.

The following callback is created by passing an array that consists of an object and a string. The method of that object will be used in the callback, and each model in the collection will be passed to the method as the first parameter. While this results in a successful callback, the doc block of the walk method of a collection incorrectly reports that it will only accept a string, while the following example uses an array.

<?php
    public function example()
    {
        /** @var \Magento\Customer\Model\ResourceModel\Customer\Collection $customerCollection */
        $customerCollection = $this->getCustomerCollection();
        $customerCollection->walk([$this, 'changeFirstname']);
        $customerCollection->save();
    }

    public function changeFirstname(\Magento\Customer\Model\Customer\Interceptor $customer)
    {
        $customer->setFirstname('Testing');
    }

Manual testing scenarios

Code is provided above to show a scenario where the doc block does not reflect what it will actually accept.

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)

@@ -492,7 +492,7 @@ public function clear()
*
* Returns array with results of callback for each item
*
* @param string $callback
* @param string|array $callback
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace to callable, I believe it is a better alternative.

As always, amend commit and do force push, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done & squashed into a single commit. Please let me know if anything else is needed.

@orlangur orlangur self-assigned this Jan 27, 2018
@orlangur
Copy link
Contributor

@ByteCreation no need to specify string, see 519754d.

Now please squash changes from 3 commits into single commit and force push. If you need assistance with that, just tell me.

- Edited doc block of the walk method in a Collection to reflect that this method will accept a callback
@ByteCreation ByteCreation force-pushed the collection-doc-block-update-2.2 branch from 519754d to 906713b Compare January 29, 2018 21:19
@orlangur orlangur added this to the January 2018 milestone Jan 30, 2018
@magento-engcom-team
Copy link
Contributor

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

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.

3 participants