search

symfony-cmf / seo-bundle

A SEO Solution for duplicate contents, page titles, etc.
https://cmf.symfony.com
47 stars 27 forks source link

Fix XSD and a simple deprecation notice #273

Closed wouterj closed 8 years ago

wouterj commented 8 years ago

This feature was implemented to default to the Model class. However, that would never be the correct value. Either the PHPCR document has to be used or one of the custom documents created by the user for their persistence needs.

As you will have hard to debug errors when this setting has a wrong value, I propose to throw an exception instead of defaulting to some faulty value.

Meanwhile, I've fixed the XSD.

/cc @ElectricMaxxx

ElectricMaxxx commented 8 years ago

Is that related to some kind of open issue? What about breaking existing applications? Those who didn't set that configuration value will get an exception now, right?

wouterj commented 8 years ago

@ElectricMaxxx this feature was introduced in 1.2-RC1, so it's released just a couple days ago. This is not part of an open issue, I think it's a more user friendly way of implementing the feature. (see https://github.com/symfony-cmf/SeoBundle/pull/249 for the feature PR)

Also, I don't think many people using the SeoBundle use it without the PHPCR persistence layer. In that case, nothing has changed.

ElectricMaxxx commented 8 years ago

Seems as you exactly hit your own exception in the tests :-) Now you really know it works :-)

wouterj commented 8 years ago

Updated the PR to just do the XSD fixing and one deprecation notice. As ORM uses the model class, defaulting to Model isn't as bad as I thought :see_no_evil:

dbu commented 8 years ago

tests complain that some xml does not match the schema.

wouterj commented 8 years ago

Fixed

dbu commented 8 years ago

:+1: