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

[Registry] Added service registry context #4836

Merged
merged 1 commit into from
Apr 21, 2016

Conversation

dantleech
Copy link
Contributor

@dantleech dantleech commented Apr 20, 2016

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets #4814
License MIT

Added additional argument to the service registry indicating the context in
order to give more helpful exception messages.

So instead of:

Service of type "boolean" does not exist.

You get:

Grid field of type "boolean" does not exist.

The context is optional in order to preserve BC (I would personally consider making it mandatory).

</service>

<service id="sylius.registry.report.renderer" class="%sylius.registry.report.class%">
<argument>Sylius\Component\Report\Renderer\RendererInterface</argument>
<argument>Renderer</argument>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

s/Renderer/Report renderer

@dantleech dantleech changed the title Added service registry context [Registry] Added service registry context Apr 20, 2016
@pjedrzejewski
Copy link
Member

👍

@dantleech
Copy link
Contributor Author

Whilst I am here, what do you think about modifying the NotExistingServiceException by passing the keys of the registered services, so the message becomes:

Grid field of type "bool" does not exist, existing grid field types: "boolean", "string", "number", "foobar"

@pjedrzejewski
Copy link
Member

@dantleech Sounds great!

- Added additional argument to the service registry indicating the
  context in order to give more helpful exception messages.
- Show available services in the "not found" exception message.
@dantleech
Copy link
Contributor Author

Updated.

@pjedrzejewski pjedrzejewski merged commit 3e0fd85 into Sylius:master Apr 21, 2016
@pjedrzejewski
Copy link
Member

Thanks @dantleech!

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
[Registry] Added service registry context
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

4 participants