search

Novactive / NovaeZSEOBundle

Novactive eZ Publish and Platform SEO Bundle
MIT License
25 stars 58 forks source link

Change config have to provide complete settings #87

Open wizhippo opened 6 years ago

wizhippo commented 6 years ago

If you want to change one setting for a sitegroup under nova_ezseo you have to redefine all the settings, This makes it very tedious if you are trying to leverage any kind of setting inheritance.

Plopix commented 4 years ago

Hey @janit @kmadejski or even @andrerom what's the correct way to achieve that. Is this correct? https://github.com/Novactive/NovaeZSEOBundle/blob/master/bundle/DependencyInjection/NovaeZSEOExtension.php#L50

Thanks!

andrerom commented 4 years ago

I'm not that deep in DI, and all the people you ping are all outside engineering. But perhaps @wizhippo or @kmadejski have suggestions on what needs to be done here.

kmadejski commented 4 years ago

I'm not sure how to make it work properly with the current approach, but... Shouldn't we look into the way how configuration is made in the first place? For instance:

nova_ezseo:
    system:
        default:
            default_metas:
                author: "eZ Community Bundle Nova eZ SEO Bundle"
                copyright: ~
                generator: "eZ Platform"
                MSSmartTagsPreventParsing: "TRUE"

Wouldn't it make more sense and be much more aligned with all other bundles if it would be like the following?

ezplatform:
    system:
        default:
            nova_ezseo:
                default_metas:
                    author: "eZ Community Bundle Nova eZ SEO Bundle"
                    copyright: ~
                    generator: "eZ Platform"
                    MSSmartTagsPreventParsing: "TRUE"

If you see what I mean and agree, then it could be done in a way that is pretty well described in the docs: https://doc.ezplatform.com/en/latest/guide/siteaccess/#exposing-siteaccess-aware-configuration-for-your-bundle. You can also grasp some real-life idea on how to make it here: https://github.com/kmadejski/ezplatform-maintenance-mode/blob/master/src/bundle/DependencyInjection/Configuration/Parser/MaintenanceMode.php

Plopix commented 4 years ago

oh I see! including it in the ezplatform kind of namespace. I see. That's probably what is wrong and maybe it did not exist from the beginning.

I will give it a try for the next version! and it will require an upgrade guide.

Thanks, everyone! Appreciated it!

Plopix commented 4 years ago

Actually I am not sure. What's inside the doc is what we have on this bundle @kmadejski can you explain a bit more?

The doc provides a way to do:

nova_ezseo:
    system:
        default:
            default_metas:
                author: "eZ Community Bundle Nova eZ SEO Bundle"
                copyright: ~
                generator: "eZ Platform"
                MSSmartTagsPreventParsing: "TRUE"

and not

ezplatform:
    system:
        default:
            nova_ezseo:

Am I wrong?

kmadejski commented 4 years ago

Hi @Plopix! Indeed, it seems that doc describes your approach actually, I was too quick with the answer, sorry. I'll try to find a bit of time during the weekend and investigate this issue. I'll keep you posted.