A partial archive of meta.discourse.org as of Tuesday July 18, 2017.

SiteSetting file format update for locale override and hidden params based on Rails.env

fantasticfears

I am working on the feature to allow overriding site setting based on the default_locale. @sam have talked with me that site setting based on environment is misleading. So I’d like to draw some proposals here to see which one is the best to go.

For the record, taking an example of the possible format:

min_search_term_length:
  client: true
  default:
    development: 2
    default: 3
  hidden:
    production: false
    development: true

As an aside, the overrides setting format will be:

min_search_term_length:
  client: true
  default: 3
  locale_default:
    zh_CN: 1
  hidden: false

hidden property proposal

hidden only accepts a true value instead of dependending on env. I don’t think a good use to hide anything in the dev and test environment. There are only 4 cases under the development category.

Rails.env params in the site setting

Remove any Rails.env params in the site setting and put them in another yaml file.

Related site settings should be put on a file and loaded with site setting. Can be config/site_settings_{dev,env}.yml

  • + The site setting file is clean.
  • + The site settings are centralized in a place so that it’s easy to follow.
  • - Another file

I’d prefer this for clarity.

Put them into an initializer

Same pros as above. It’s not that obvious if they are put in config/initializers/005-site_settings.rb.

Merge them with locale_default

Instead of sending them outside the site setting files, we can have a override params to put locale or env.

min_search_term_length:
  client: true
  default: 3
  default_overrides:
    development: 2
    zh_CN: 1

This style also looks good considering such overrides are not much.

erlend_sh

@team any input here?

sam

My gut feel here is to clean up site setting yml file so it only includes per locale settings and does not have any of the per-env settings.

Otherwise what happens is that devs need to start reasoning about precedence.

  • Do env default overrides happen prior to locale overrides?
  • Do we need per env per locale overrides?

It gets super complicated fast, so my call here is just to add an initializer that does all the per-env stuff and keep the yml file so it only contains per-locale default overrides.

@eviltrout what do you think?

eviltrout

I definitely think it’s simpler not to include per-env settings in the file.

I think it’s reasonable to add per-locale to the file then follow this order:

  1. Always use override if present
  2. If not overridden, use locale specific
  3. If no locale specific, use default
sam

OK,

@fantasticfears so we have a call here.

  • Remove all per-env settings from file, move the logic for per-env overrides into development.rb and test.rb , so you got to make sure that the API still supports the overriding.

  • Go with the locale_default syntax.

  • Make sure there are lots of tests for this stuff, it is super critical code.