While getting drupal 8 ready, we need to have default config settings yml files, and we are making the saved active config files match in format so that a diff between them means that a change in configuration has actually happened. We dont want a diff to show a diff if the format is just different but the data is the same.

There is no coding standards for config yml yet.

Proposed resolution

Eventually make a standards doc page to put

Remaining tasks

  • Gather the conventions that are currently being used and document them.
  • Discuss what the standard will be and link to related issues.

User interface changes

No UI changes.

API changes

No api changes.

Related Issues


jhodgdon’s picture

It seems like if the goal is that a written-out config file and the one saved in core are the same, the solution would be to make sure before we commit any YML files to core, we have the core-writer-of-yml function write them out? :)

Does the core writer preserve comments that were in the original YML though (I think at least that YML can have comments)?

And is this issue about making a standard for the core-writer-of-yml functionality, or just about making a standard for writing YML documents?

YesCT’s picture

I'd suggest this issue is about making a standard for the core-writer-of-yml functionality.
And that we start by just writing in words what that is doing right now.
For example:
single quoting strings with whitespace
ordering items by.. (how does it order)?
indenting by 2 spaces and not tabs
how does it represent empty lists (sequences)
linking to the (api) page that is the writer code so it is easy to find.

does it: do keyed lists like
book: '0'
or just not list it
does it do
book: 'book'
or just
- book

I think comments are stripped out from the default config when the active config is written. And then diffs are done on if config has changed between the old active config and the new active config (or maybe staged config) and none of those have comments. (I'm not sure if #1602106-4: Document default configuration files is still accurate.)

This issue, the standard, would be helpful for people who are writing the config writer to make sure it is writing in the format we want it to. I think there might not be just one config writer, but different ones for different areas. For example,
#1933548: Book allowed_types settings repetitive and in under certain conditions can change unexpectedly
book: book
- book

but then I ran into
article: article
in #1948884-14: Create configuration schemas for block and custom_block modules 7.

Maybe that is a problem with how the data is stored and not the config writer... but I only notice it when looking at the yml.

We might also want some kind of check list.. like:
if you change the way data is stored:
check the config written to the yml files if it matches (this) standard
write also the default config, add back comments and replace the default config yml file
check the schema to make sure changes to the way data is stored are also reflected in the schema.

jhodgdon’s picture

This doesn't sound like a coding standard to me, exactly. Our coding standards are about syntax, whitespace, and other mechanical issues in how to write readable, maintainable code....

And I'm a bit confused. Why would people be writing new writer-of-config classes anyway?

Gábor Hojtsy’s picture

Yeah, well, the standard is "whatever is output by the core yml writer when you create the yml with core". But if we don't have the rules of that written down, then the code review for shipped configuration would be to go and generate it yourself and see if it matches whatever in the patch. May be more tedious then looking through a checklist of things and visually acknowledging it would match? Are you advocating not documenting the format standards but instead of rely on dynamic verification only?

jhodgdon’s picture

But... As discussed above, we probably want the files shipped with core to be hand-written and contain comments. So they're never going to match the machine-generated output, which strips the comments.

This other issue is specifically about standards for documenting the hand-written YML files:
#1602106: Document default configuration files

And when I asked above, I was told that this issue is about standards for the machine-written YML files only...

So... Let's figure out what the issue is about, and if it's not applicable to hand-written YML files then I think the docs should go on the interface that all YML writers implement (assuming there is one) (since it's part of the interface contract in that case, right?).

Gábor Hojtsy’s picture

I'm surprised we want to document shipped config because we also want diffs for config changes to be meaningful. If the order of items, type of items and inclusion/lack of comments is not the same, then the diffs of changed/staged config will be pretty overwhelming and not very indicative of actual changes.

jhodgdon’s picture

The question of putting docs into shipped config should be discussed on that other issue.

Gábor Hojtsy’s picture

All right, so the question in this issue is whether to impose/document any standards on hand-written .yml (excluding thinking of comments) to make it easier to review although it woud be re-saved into active config (in a possibly different format) is worthwhile. Or people should write their hand written shipped config in whatever style and we don't care about that.

Gábor Hojtsy’s picture

Discussion from IRC:

[5:07pm] jhodgdon: GaborHojtsy: I was told earlier on that issue that it's about standards for how YML writers should write out YML, not about how humans should write YML.
[5:07pm] jhodgdon: GaborHojtsy: It sounded to me (I'm not familiar with the YML system yet) like the config writer would early on (during install?) write out YML files itself, so after that you could diff the initial YML with config changes?
[5:10pm] GaborHojtsy: jhodgdon: well, its documenting how the machine already does it
[5:10pm] GaborHojtsy: jhodgdon: "I'd suggest this issue is about making a standard for the core-writer-of-yml functionality. And that we start by **just writing in words what that is doing right now.**"
[5:11pm] GaborHojtsy: jhodgdon: quoting YesCT|museum
[5:11pm] GaborHojtsy: jhodgdon: emphasis mine
[5:11pm] jhodgdon: GaborHojtsy: yeah, I don't think it needs to be documented really but whatever. If we want people to follow the same standards then we should just say "Write out the config and that is the one to save in the repo"
[5:11pm] GaborHojtsy: jhodgdon: well, not because you want to document it
[5:11pm] GaborHojtsy: jhodgdon: so if you take the core file, then write it out, then submit as a patch, that will remove comments
[5:12pm] jhodgdon: GaborHojtsy: exactly. It's tedious for people to try to replicate machine standards exactly, and if there will be diffs anyway, then maybe we should not put in comments if those diffs are important.
[5:12pm] jhodgdon: and if they aren't important, we shouldn't worry too much about the standards
[5:12pm] GaborHojtsy: jhodgdon: also then we did not answer the "I as a patch reviewer want a set of rules I can review a .yml file by so I don't need to feed it into Drupal for it to spit it back out and needing me to diff manually"
[5:13pm] Crell: Did I just see a user story in #drupal-contribute?
[5:13pm] GaborHojtsy: jhodgdon: I think it will end up with changes back and forth, eg. values: [a, b, c] is the same as
[5:13pm] GaborHojtsy: values:
[5:13pm] GaborHojtsy: - a
[5:13pm] GaborHojtsy: - b
[5:13pm] GaborHojtsy: - c
[5:13pm] GaborHojtsy: (I think)
[5:14pm] jhodgdon: GaborHojtsy: I'm all in favor of standards, but if the objective is to not have diffs, then the standard should be "write it out with the machine"
[5:14pm] msonnabaum: GaborHojtsy: it is, it'd be great to not standardize that tho
[5:14pm] GaborHojtsy: jhodgdon: so we have some hand written config with […] and when someone will take the spit-out version and submit as patch, it will change to something else, and then somebody will shorten it for easier reading and then ...
[5:14pm] jhodgdon: GaborHojtsy: if the objective is human readability, then we should make some standards and not worry about diffs.
[5:14pm] msonnabaum: jhodgdon: GaborHojtsy: also keep in mind, Symfony's yaml parser accepts invalid yaml, which we already have in our repo
[5:15pm] jhodgdon: oh great, at least that should be a standard!
[5:15pm] jhodgdon: msonnabaum: I mean, we should at least have a standard that our YML is valid, shouldn't we?!?
[5:15pm] msonnabaum: jhodgdon: totally agreed there
[5:15pm] msonnabaum: jhodgdon: just saying, we can't rely on symfony for that
[5:16pm] jhodgdon: msonnabaum: won't the writer write it out as valid YML?
[5:16pm] GaborHojtsy: jhodgdon: I want to avoid the cycles of readability cleanup => pure spit-out knock-over => readability cleanup => ...
[5:16pm] msonnabaum: jhodgdon: not sure, I kinda doubt it
[5:16pm] msonnabaum: jhodgdon: for example, we have strings that start with @ with no quotes, symfony is ok with that, but it's not valid
[5:17pm] GaborHojtsy: jhodgdon: so long as we are fine with those cycles, the core committers will one point question why are we going back and forth and we'll need to point somewhere which one is better

(ended there).

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.