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

Code generate: support variadic parameter #10771

Merged
merged 5 commits into from
Sep 8, 2017
Merged

Code generate: support variadic parameter #10771

merged 5 commits into from
Sep 8, 2017

Conversation

kenboy
Copy link
Member

@kenboy kenboy commented Sep 5, 2017

Support variadic parameter

Description

Fix error:
Fatal error: Declaration of TNW\Salesforce\Synchronize\Unit\Customer\Account\Lookup\ByName\Interceptor::get($path, $objects) must be compatible with TNW\Salesforce\Synchronize\Unit\UnitInterface::get($path, ...$objects)

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 Sep 5, 2017

CLA assistant check
All committers have signed the CLA.

@ishakhsuvarov
Copy link
Contributor

Hi @kenboy
Please consider updating the tests for class generator to support this case.
Also, please sign the CLA with the email you used for the commit.
Thank you

@ishakhsuvarov ishakhsuvarov self-assigned this Sep 5, 2017
@ishakhsuvarov ishakhsuvarov added this to the September 2017 milestone Sep 5, 2017
@kenboy
Copy link
Member Author

kenboy commented Sep 5, 2017

Hi @ishakhsuvarov
Completed

@ishakhsuvarov
Copy link
Contributor

@kenboy Thank you

/**
* {@inheritdoc}
*/
public function variadic(... $values)
Copy link
Contributor

@ishakhsuvarov ishakhsuvarov Sep 6, 2017

Choose a reason for hiding this comment

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

It would be nice to extend this case to support multiple arguments before variadic, and type-hinted variadic (something in the lines of zend-code test asset).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do you think that space should be used after ...? As far as I can see it is usually omitted.

Copy link
Member Author

Choose a reason for hiding this comment

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

@kenboy
Copy link
Member Author

kenboy commented Sep 7, 2017

Hi @ishakhsuvarov
Added support multiple arguments before variadic, and type-hinted variadic

@magento-team magento-team merged commit 415eb03 into magento:develop Sep 8, 2017
magento-team pushed a commit that referenced this pull request Sep 8, 2017
[EngCom] Public Pull Requests
 - MAGETWO-72390: Remove the usage of the DataObject for response management #10808
 - MAGETWO-71545: Added 'application/json' Content-Type to Ajax responses in the Magento_UI module. #10521
 - MAGETWO-72388: Fix spelling mistake in AddressTest.php #10806
 - MAGETWO-72283: Code generate: support variadic parameter #10771
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