Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

More explicit name for requested name #5755

Merged
merged 4 commits into from Feb 19, 2014

Conversation

Projects
None yet
3 participants
Contributor

jmleroux commented Jan 27, 2014

No description provided.

@jmleroux jmleroux and 2 others commented on an outdated diff Jan 27, 2014

library/Zend/Form/FormAbstractServiceFactory.php
* @return bool
*/
- public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $rName)
+ public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $requestedName)
{
@jmleroux

jmleroux Jan 27, 2014

Contributor

I take this opportunity to ask for the utility of $name variable.

I searched all ZF2 abstract factories, and this variable is never used.

github noob question : is there a way to discuss a line of code without submiting a PR ?

@Maks3w

Maks3w Feb 15, 2014

Member

Sending a mail to the list is the best option. Another one is add a comment in the commit which add that change but we don't recommend that option.

@Ocramius

Ocramius Feb 15, 2014

Member

@jmleroux you can also just link the line of code by clicking on it and pressingY to go to the canonical URI for it.

The mailing list is the best place for discussions indeed

@Maks3w Maks3w commented on an outdated diff Feb 15, 2014

library/Zend/Form/FormAbstractServiceFactory.php
* @return bool
*/
- public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $rName)
+ public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $requestedName)
@Maks3w

Maks3w Feb 15, 2014

Member

Could you rename $services with $serviceLocator by this way the signature match exactly with the interface.

@Maks3w Maks3w added a commit that referenced this pull request Feb 19, 2014

@Maks3w Maks3w Merge pull request #5755 3cd0e16

@Maks3w Maks3w added a commit that referenced this pull request Feb 19, 2014

@Maks3w Maks3w Merge pull request #5755 in develop 07a397b

@Maks3w Maks3w added a commit that referenced this pull request Feb 19, 2014

@Maks3w Maks3w Merge pull request #5755 in master 1af6ed6

@Maks3w Maks3w merged commit 8c485ad into zendframework:master Feb 19, 2014

1 check passed

default The Travis CI build passed
Details

@jmleroux jmleroux deleted the jmleroux:patch-3 branch Feb 20, 2014

@gianarb gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015

@Maks3w Maks3w Merge pull request zendframework/zendframework#5755 e450dc0

@gianarb gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015

@Maks3w Maks3w Merge pull request zendframework/zendframework#5755 in develop 9ed9a14

@gianarb gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015

@Maks3w Maks3w Merge pull request zendframework/zendframework#5755 in master bdd4b7d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment