Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

More explicit name for requested name #5755

Merged
merged 4 commits into from
Feb 19, 2014
Merged

More explicit name for requested name #5755

merged 4 commits into from
Feb 19, 2014

Conversation

jmleroux
Copy link
Contributor

No description provided.

* @return bool
*/
public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $rName)
public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $requestedName)
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 ?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@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

* @return bool
*/
public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $rName)
public function canCreateServiceWithName(ServiceLocatorInterface $services, $name, $requestedName)
Copy link
Member

Choose a reason for hiding this comment

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

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

Maks3w added a commit that referenced this pull request Feb 19, 2014
Maks3w added a commit that referenced this pull request Feb 19, 2014
Maks3w added a commit that referenced this pull request Feb 19, 2014
@Maks3w Maks3w merged commit 8c485ad into zendframework:master Feb 19, 2014
@jmleroux jmleroux deleted the patch-3 branch February 20, 2014 07:42
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants