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 syslog module.
Schema in place
Schema not yet in place
syslog.settings.yml
Comment | File | Size | Author |
---|---|---|---|
#13 | 1919206-syslog-schema-11.patch | 565 bytes | vijaycs85 |
#13 | 1919206-diff-9-11.txt | 431 bytes | vijaycs85 |
#9 | 1919206-syslog-schema-9.patch | 558 bytes | vijaycs85 |
#9 | 1919206-diff-4-9.txt | 423 bytes | vijaycs85 |
#6 | 2013-02-21_231646.png | 12.65 KB | vijaycs85 |
Comments
Comment #1
vijaycs85Adding schema file...
Comment #3
vijaycs85Comment #4
vijaycs85Updating patch with code style fix and labels...
Comment #5
YesCT CreditAttribution: YesCT commentedI'm going to review this for http://drupal.org/node/1905070#codestyle
Comment #6
vijaycs85Adding screenshot...
Comment #7
itarato CreditAttribution: itarato commentedHi,
I've applied the patch, then I re-installed my D8 site, enabled syslog module, generated couple of log messages and it worked fine.
Comment #8
YesCT CreditAttribution: YesCT commentedthis is following the pattern in http://drupal.org/node/1905070#codestyle
but I think the example there might be missing an article:
# Schema for configuration files of contact module.
How about:
# Schema configuration files for the contact module.
Comment #9
vijaycs85Sure, why not :)
Comment #10
YesCT CreditAttribution: YesCT commentedI wonder if there is an issue for config standards. until I find it. I'm going to note here (since this is one of the first being reviewed) that http://drupal.org/node/1354#files uses the article "the" in http://drupal.org/node/1354#files " * Install, update, and uninstall functions for the XXX module."
And a file really does not contain files...
so:
should maybe be:
Schema configuration file for the syslog module.
or just:
Schema configuration for the syslog module.
or maybe:
Configuration for the schema for the syslog module.
or maybe:
Configuration file for the schema for the syslog module.
Let me ponder a minute. and get someone else to read it.
Comment #11
YesCT CreditAttribution: YesCT commentedvijay is helping me in irc understand.
this was helpful for me:
Schema (system.schema.yml) for configuration files(system.site.yml, system.theme.yml, etc) of system module.
So:
# Schema for the configuration files of the system module.
is probably accurate (similar to what was originally there, but with some "the" articles).
Comment #12
YesCT CreditAttribution: YesCT commentedsounds contradictory.
never use quotes, then use a quote even if they are one word...
I need to read that again.
Comment #13
vijaycs85Updating accordingly...
Comment #14
YesCT CreditAttribution: YesCT commentedI did a standards review. and this looks rtbc to me, with the above functional review.
we should do a few more to see if the standards are working out.
still a nagging wonder if the comment should be:
any one actually know english grammar?
[09:45am] YesCT: should it have a "the" in:
[09:45am] YesCT: Schema for the configuration files of the syslog module.
[09:45am] YesCT: or not:
[09:45am] YesCT: Schema for configuration files of the syslog module.
lets leave it as is in the patch.
----
the indenting looks right.
in the standard it says:
I'll think about that. the wording in the standard might be tweaked/improved.
I think it's trying to say that the spacing is important because it controls the *meaning* not just making it look nice and be consistant.
Comment #15
YesCT CreditAttribution: YesCT commentedI updated http://drupal.org/node/1905070#codestyle with the one line comment that matches what this patch is doing, with the the.
Comment #16
webchickCommitted and pushed to 8.x. Thanks!