backdrop-contrib / simplenews

BackdropCMS port of the Simplenews module for Drupal 7
GNU General Public License v2.0
1 stars 3 forks source link

Get rid of the module name as prefix in config variables #4

Closed alanmels closed 4 years ago

alanmels commented 5 years ago

Just to put this to the issue queue. Copied from Gitter November 8, 2019 11:33 PM:

@docwilmot: @alanmels we tend to not include the module name as prefix in config variables, like "simplenews_news" would change to just "news. The prefix was necessary with D7 variables as they were all in one big table but since they're all inn individual files in Backdrop, not needed. But not a problem either way. Just FYI.

@herbdool: @docwilmot @alanmels though I think because we reintroduced the option of system_settings_form() it means the form element name becomes the config name. If I'm correct

@alanmels: @docwilmot, the module was first processed through the coder_upgrade module, which has done most of the job, including putting those prefixes. My goal was to get the module work sooner, so the code has not been brought up to Backport standards/practices. But thanks a lot for pointing it out, I'll see to if we can get rid of the prefixes in one of the next releases soon. For several days we'll be busy with maintenance workload in AltaGrade, taking part only in quick tasks for Backdrop.

@docwilmot: @alanmels this is not a rule, just a general trend. i recall we discussed this really early years ago and it just stuck.

@herbdool: @docwilmot I started off doing it that way but wherever it uses system_settings_form can't do it that way (not without changing the form keys as well)

@docwilmot: i change the form keys too

@quicksketch: If I were writing a new module I wouldn't add the prefixes, it's redundant with the config file name. So usually I change the config and form keys when upgrading a module to do the same.

alanmels commented 4 years ago

Cleaning up the queue... Since it's working fine, I'll leave prefixes intact for now.