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

[ContainerBuilder] getDefinitionByType() method added #137

Merged
merged 1 commit into from Jan 7, 2017
Merged

[ContainerBuilder] getDefinitionByType() method added #137

merged 1 commit into from Jan 7, 2017

Conversation

TomasVotruba
Copy link
Contributor

All is described in the issue: #130

Behavior is compatible with with getDefiniton() - exception is thrown when not found.
This method is called in cases where Definition is required.

@TomasVotruba
Copy link
Contributor Author

Ping @dg

@TomasVotruba
Copy link
Contributor Author

TomasVotruba commented Jan 7, 2017

What needs to be done here?

@dg
Copy link
Member

dg commented Jan 7, 2017

Can it return NULL?

@TomasVotruba
Copy link
Contributor Author

TomasVotruba commented Jan 7, 2017

Well, that was my first idea. But having getDefiniton() throwing exception and getDefinitonByType() giving NULL seems inconsistent.

What do you think?

@dg
Copy link
Member

dg commented Jan 7, 2017

So annotation @return should be fixed.

@TomasVotruba
Copy link
Contributor Author

I see. That is leftover from 1st idea.

Fixed!

@dg
Copy link
Member

dg commented Jan 7, 2017

Great, thanks!

@dg dg merged commit 5ea207b into nette:master Jan 7, 2017
@TomasVotruba
Copy link
Contributor Author

Awesome, thanks a lot!

@TomasVotruba
Copy link
Contributor Author

For others, here are all use cases I used it in:

deprecated-packages/symplify@1e8854c

Requires nette\di 2.4.6+

* Gets the service definition of the specified type.
* @param string
* @return ServiceDefinition|NULL
*/
Copy link
Member

Choose a reason for hiding this comment

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

Can it return NULL?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants