Skip to content

Conversation

@vovayatsyuk
Copy link
Member

Description

addIdentity method accepts strings, not an array.

See the usage:

  1. https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Catalog/Plugin/Block/Topmenu.php#L119
  2. https://github.com/magento/magento2/blob/2.2-develop/app/code/Magento/Catalog/Plugin/Block/Topmenu.php#L129

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)

@dmanners dmanners self-assigned this Nov 30, 2017
@dmanners dmanners added this to the November 2017 milestone Nov 30, 2017
* Add identity
*
* @param array $identity
* @param string $identity
Copy link
Contributor

Choose a reason for hiding this comment

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

technically this method accepts both strings and arrays as in_array will work with both and we never actually check the type. Since this class is noted with the @api I think it would be best to update the typehint to be string|array as we do not know how people use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, never know that I can use array as identity.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean technically you can, not sure you should do though.

@okorshenko okorshenko merged commit 80520c5 into magento:2.2-develop Dec 1, 2017
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