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 language module.
Schema in place
Schema not yet in place
language.detection.yml
language.mappings.yml
language.negotiation.yml
Comment | File | Size | Author |
---|---|---|---|
#7 | 1919178-language-detection.png | 30.01 KB | sandipmkhairnar |
#7 | 1919178-language-detection2.png | 11.92 KB | sandipmkhairnar |
#7 | 1919178-language-detection3.png | 13.68 KB | sandipmkhairnar |
#7 | 1919178-language-schema-7.patch | 1.41 KB | sandipmkhairnar |
#7 | 1919178-diff-6-7.txt | 1.41 KB | sandipmkhairnar |
Comments
Comment #1
vijaycs85Adding schema file...
Comment #2
vijaycs85Updating patch with label and code style changes.
Comment #4
vijaycs85Adding config inspector for review...
Comment #5
YesCT CreditAttribution: YesCT commentedthe tests part of #2 might have been included by accident. or included in another patch recently.
it's already in a fresh git pull --rebase
schema for langcodes
there are three files in the sites/modules/language/config
1. Do the mappings there (for chinese langcodes) need to be in the schema?
There is a ui for them now:
because of #365615: Language detection not working correctly for most Chinese readers (and add a user interface for all browser language mappings)
indent level
orig settings:
on save, in the config hash in the sites default files dir, the indent is different:
in the schema, the indent implies it might be further...
2. should we worry about that?
bit more match to ui labels
looking at the detection ui, I think some of the labels can be changed to match that.
3. I dont see schema for the other settings besides url and session though. Do we need those too?
I changed the detection to check more, but I did not see any difference in the saved config in the sites default config hash language file.
the session parameter label matches the schema
as does the detection label for session... but for url, it was (part of) the description. I made it include the rest of the description.
4. but probably they should both be the descriptions, or both just be the method name, right?
(patch attached just tries to get the labels matching better. the one from 2 did not apply with the tests stuff, so the interdiff doesn't show that deleted.. cause it's really still there already)
Comment #6
vijaycs85Thanks for the review @YesCT.
#5.1 - If we need to map them, then we can have something like(not tested):
#5.2 - guess spacing is important in yml. Created #1934428: Remove spacing issue in language.negotiation.yml to fix this problem.
#5.3 - we add schema only for config in yml file of the module(meaning, may not cover the stuff that's getting added dynamically in active dir).
patch in #5 looks great, except ending line (also update language in comment to Language, if line ending update needed.)
Comment #7
sandipmkhairnar CreditAttribution: sandipmkhairnar commentedThanks @vijaycs85 updated patch as per comment.
Comment #8
vijaycs85Thanks for the patch @sandipmkhairnar. Lets wait for YesCT/@gobar to review. I'm not very sure, we want to have this in Schema as we got nothing to translate.
Comment #9
tstoecklerRe #8: We need schemas for all config, regardless of whether it is translatable or not.
Comment #10
vijaycs85Thanks the clarification @tstoeckler. Patch in #7 is ready for review now...
Comment #11
YesCT CreditAttribution: YesCT commentedthe interdiff is the same as the patch. :)
I didn't retest this manually with the config inspector. it looks like that was done already.
The code looks good schema standards-wise.
And I compared it to the config files and it looks like it matches.
Comment #12
webchickCommitted and pushed to 8.x. Thanks!