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

Change config have to provide complete settings #87

Open
wizhippo opened this issue Oct 23, 2018 · 6 comments
Open

Change config have to provide complete settings #87

wizhippo opened this issue Oct 23, 2018 · 6 comments

Comments

@wizhippo
Copy link
Contributor

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
Copy link
Contributor

Plopix commented Jul 14, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

Plopix commented Jul 16, 2020

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
Copy link
Contributor

Plopix commented Jul 16, 2020

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants