webfactory / exceptions-bundle

A bundle to ease development of custom, user-friendly Symfony2 error pages.
MIT License
62 stars 15 forks source link

SensioFrameworkExtraBundle is required #8

Closed pitpit closed 9 years ago

pitpit commented 10 years ago

It may be mention in README and you should also add sensio/framework-extra-bundle into composer.json requirements. Thank you :)

mpdude commented 10 years ago

Is that only for the annotations in the TestController?

If so, IMO we should find another way for that so we don't need the dependency at all.

@MalteWunsch what do you think?

MalteWunsch commented 10 years ago

Annotations in the test controller + the annotation syntax in the routing.yml discussed in the referencing case. That's it, as far as I can see - we should remove it, agreed.

mpdude commented 10 years ago

We could provide yml and xml files that contain the route definition, so folks could include one of those in their routing files.

That would slightly change the way the bundle has to be set up, but it would not be a BC break as long as we keep the annotations. (Note that we do no longer need to require the framework-extra-bundle though!)

MalteWunsch commented 10 years ago

I don't think we should keep annotations without providing the ability to read them. It confuses both people (why don't these annotations work?) and machines (static analysis tools).

If we take the Route annotation from the TestController and write it as the path into the the routing.yml, where is the a BC break?

MalteWunsch commented 10 years ago

routing.yml to something like this:

webfactory_exceptions:
    path:      /_error/{code}/{_format}
    defaults:  { _controller: AcmeBlogBundle:Blog:show, _format: "html" }
    requirements:
        code:  \d+

(Slight BC break in probably changing the route name, but we can solve that easily)

mpdude commented 10 years ago

My bad!

I was under the impression that our instructions were telling people to register the Controller using the annotation type. Then, taking away these annotations would be a break for existing setups.

But in fact, we're telling to simply import the routing.yml, so go for it :+1: I don't think we need to care about the route name, simply chose a sensible one.