0011 - Use constants for configuration variables

[JevgenijVisockij]

No description provided.

[matks]

ADR added in table https://github.com/PrestaShop/ADR/blob/master/README.md

[matks]

Could you provide a few examples of such configuration names and how they are used in the code ? (just to make it easier for reader to understand the topic, this ADR also serves as documentation for people in the future)

[JevgenijVisockij]

Updated the PR to include examples.

[matks]

To be honest, we must also list the drawbacks 😄 . This means for example we need to require or use the Configuration class everywhere.

[JevgenijVisockij]

Added 2 drawbacks, couldn't think of more at the moment.

[matks]

PR looks good to me. Complete. Ready to discuss and vote.

[matks]

@PrestaShop/prestashop-core-developers Can you check this and ask questions if you have? Else we can start the voting phase.

[Progi1984]

@matks You should ping @PrestaShop/prestashop-maintainers. May we should create a meeting for all ADRs?

[matthieu-rolland]

Being working on the configuration forms at the moment, I wholeheartedly agree with this ADR.

About the drawbacks, they are minor in my opinion, this could be implemented progressively.

[matthieu-rolland]

Also, configuration names don't always match the names of configuration form fields, for example, form field enable_shop is linked to the configuration name PS_SHOP_ENABLE. (and not PS_ENABLE_SHOP as we would intuitively guess).

Using constants would make it less error-prone.

[kpodemski]

I like the idea, I'm not sure if main Configuration class is good for those statics,

Configuration::get(Configuration:PS_SHOP_NAME);
`Configuration::get(ConfigKey::PS_SHOP_NAME);

use ConfigurationKeys as Key;
`Configuration::get(Key::PS_SHOP_NAME);

I don't think it's that important, I'm thinking aloud :)

[PierreRambaud]

I like the idea, I'm not sure if main Configuration class is good for those statics,

Configuration::get(Configuration:PS_SHOP_NAME);
`Configuration::get(ConfigKey::PS_SHOP_NAME);

use ConfigurationKeys as Key;
`Configuration::get(Key::PS_SHOP_NAME);

I don't think it's that important, I'm thinking aloud :)

As I said in the PR, I'm like @kpodemski, using ConfigurationCore class isn't the best one (imho). I'm not even sure about the ConfigurationKeys class too 😅 but maybe in the adapter?

[PierreRambaud]

Hi @PrestaShop/prestashop-maintainers , I'm not sure it's a good idea to keep ADR for months. If it's not relevant, maybe it's time to close it? 🤔

[matks]

Yes :( unfortunately ADR are a costly process as it requires time from each maintainer to explore and discuss the topic, so it's hard to be able to get everybody's attention and investment on a single topic. This topic did not make it.