This is a sub-issue of #1910624: [META] Introduce and complete configuration schemas in all of core.

Problem/motivation

#1866610: Introduce Kwalify-inspired schema format for configuration introduced the idea of config schema. The changelog leads to (hopefully extensive) documentation on the format at http://drupal.org/node/1905070. While there are little cleanups planned for the format overall, the current format is a result of months of back and forths, so it should be perfectly fine to apply it more widely to core.

Proposed solution

Create a configuration schema for locale module.

Schema in place

Schema not yet in place
locale.settings.yml

CommentFileSizeAuthor
#10 1919180-locale-schema-10.patch1.55 KBsandipmkhairnar
#10 1919180-diff-7-10.txt1.55 KBsandipmkhairnar
#10 lacale-schema-form.png46.37 KBsandipmkhairnar
#7 1919180-locale-schema-7.patch1.58 KBvijaycs85
#5 1919180-locale-schema-5.patch1.58 KBvijaycs85
#3 1919180-locale-schema-3.patch2.53 KBvijaycs85
#2 1919180-locale-schema.png26.64 KBsandipmkhairnar
#2 1919180-locale-schema-2.patch1.58 KBsandipmkhairnar
#1 1919180-locale-schema-1.patch965 bytesvijaycs85

Comments

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Active » Needs review
StatusFileSize
new 1919180-locale-schema-1.patch965 bytes

Adding schema file...

sandipmkhairnar’s picture

sandipmkhairnar commented
StatusFileSize
new 1919180-locale-schema-2.patch1.58 KB
new 1919180-locale-schema.png26.64 KB

updating schema as per code style in http://drupal.org/node/1905070#codestyle and verified in config_inspectorschema form

vijaycs85’s picture

vijaycs85
Location London, UK
commented
StatusFileSize
new 1919180-locale-schema-3.patch2.53 KB

Removing existing schema file and minor comment update.

Status: Needs review » Needs work

The last submitted patch, 1919180-locale-schema-3.patch, failed testing.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
StatusFileSize
new 1919180-locale-schema-5.patch1.58 KB

Updating right patch file...

guy_schneerson’s picture

guy_schneerson commented
Status: Needs review » Needs work

@vijaycs85 Looks great no syntax / Code style issues.

I am not over familiar with all the settings but all but one label look sensible.

The only change I would make is change "Strings cached" to "Cache strings".

vijaycs85’s picture

vijaycs85
Location London, UK
commented
StatusFileSize
new 1919180-locale-schema-7.patch1.58 KB

Thanks @guy_schneerson. Updated label.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
guy_schneerson’s picture

guy_schneerson commented
Status: Needs review » Needs work

Hi @vijaycs85 thanks for the updated patch.
I have reviewed all the settings and looked at the code implementing them to make sure they describe what they do.
I have located one more issue with the overwrite_customized & overwrite_not_customized.
Those don't match the UI code and don't match the implementation as far as I can see, I actually think the UI code is incorrect and will raise a separate issue.

According to the local PoDatabaseWrite.php

* - not_customized: boolean indicating that not customized strings should
* be overwritten.
* - customized: boolean indicating that customized strings should be
* overwritten.

and according to Gettext.php

'customized': Flag indicating whether the strings imported from $file
* are customized translations or come from a community source.

I think the safest approach is to go with

<?php
        overwrite_customized:
          type: boolean
          label: 'Overwrite customized translations'
        overwrite_not_customized:
          type: boolean
          label: 'Overwrite non customized translations'
?>
sandipmkhairnar’s picture

sandipmkhairnar commented
StatusFileSize
new lacale-schema-form.png46.37 KB
new 1919180-diff-7-10.txt1.55 KB
new 1919180-locale-schema-10.patch1.55 KB

Thanks @guy_schneerson for comments. Updated patch as per comment.

schema form

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs work » Needs review
guy_schneerson’s picture

guy_schneerson commented
Status: Needs review » Needs work

Thanks @vijaycs85 looks good will give it another proper test tomorrow I also posted a related issue Configuration of Import behavior may be unclear / incorect I don't think it directly relates to this patch at the moment as the new labels take a "safe" approach.

guy_schneerson’s picture

guy_schneerson commented
Status: Needs work » Reviewed & tested by the community

Looks good to me.
We have one potential small issue where all the settings under the "translation" section are all related to the "translation import" but that may be coincidental and other settings may be added to this section in the future.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x, thanks! I'll push once testbot has caught up a bit.

Automatically closed -- issue fixed for 2 weeks with no activity.

rych’s picture

rych commented

I've found an item in locale.settings.yml, but it doesn't appear in schema file.
Related issue: Complete Configuration schemas for locale module