search

symfony-cmf / seo-bundle

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

make burgov/key-value optional #318

Closed dbu closed 7 years ago

ElectricMaxxx commented 7 years ago

mh, we had that discussion when introducing that exception in our extension class. We said just not displaying the form is a silent error no one would recognize, why is it different now?

dbu commented 7 years ago

making it a hard dependency seems just wrong to me. in menu admin, we have a special configuration that is off by default that needs burgov. we only throw the exception if that config is on and burgov missing. we could do something similar here. and as seo is already relying on the form type, we would get rid of the problem completely in the sonata integration.

dbu commented 7 years ago

okay, changed this a bit. if you agree with the new logic, i will adjust the tests as well. they currently expect the old behaviour

dbu commented 7 years ago

ping @ElectricMaxxx what do you think about the code? if you agree i will adjust the tests to the new format and update the changelog.

ElectricMaxxx commented 7 years ago

Activated StyleCI and fixed against master, can you rebase please?

dbu commented 7 years ago

rebased and fixed style issues. this is now ready to merge if the tests agree.

ElectricMaxxx commented 7 years ago

@dbu can you please add some ExtensionTests for setting storage and options and ad it to the scheme file (with tests too)

ElectricMaxxx commented 7 years ago

@dbu can you do this one as second on your friday?

dbu commented 7 years ago

fixed the existing tests, added some more and then fixed the stuff the tests reveiled. hope i got it right with the fixxmlconfig?