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 #13374

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.

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

Choose a reason for hiding this comment

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

It can even be just callable (without string).

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

- 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.3 branch from 7f7103c to 6e50ae7 Compare January 29, 2018 21:13
@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.

4 participants