Problem

  • The indentation of sub-keys with 4 spaces in generated YAML files is inconsistent with Drupal's coding standards.
  • This special case was a constant confusion for novices working on config conversion patches.

Goal

  • Consistently indent with 2 spaces.

Proposed solution

  1. Commit #1713564: Make Config\FileStorage instantiate Yaml\Dumper and Yaml\Parser only once
  2. Leverage @sun's enhancement to Symfony's Yaml component to set the indentation level to 2 in Config\FileStorage. (HEAD contains this change already)
  3. Update all existing .yml files for the new indentation.
Files: 
CommentFileSizeAuthor
#6 drupal8.config-indentation.6.patch10.16 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 41,108 pass(es). View
#2 drupal8.config-indentation.2.patch9.73 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 41,099 pass(es). View
#1 drupal8.config-indentation.1.patch628 bytessun
PASSED: [[SimpleTest]]: [MySQL] 41,087 pass(es). View

Comments

sun’s picture

Status: Active » Needs review
Issue tags: +Novice, +Config novice
FileSize
628 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,087 pass(es). View

Remaining task is 3: Update all existing .yml files from 4 spaces to 2 spaces for nested keys.

dagmar’s picture

FileSize
9.73 KB
PASSED: [[SimpleTest]]: [MySQL] 41,099 pass(es). View

Here is the patch with the spaces converted. I didn't changed the .yml files inside core/vendor/* should I change those files too?

sun’s picture

Status: Needs review » Reviewed & tested by the community

Excellent, thanks @dagmar! :)

sun’s picture

#2: drupal8.config-indentation.2.patch queued for re-testing.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

I just committed #1705748: Convert simpletest settings to configuration system so I think this will need a small re-roll. Otherwise, looks good.

dagmar’s picture

Status: Needs work » Needs review
FileSize
10.16 KB
PASSED: [[SimpleTest]]: [MySQL] 41,108 pass(es). View

Re-rolled including the simpletest config file.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the quick re-roll, @dagmar!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, awesome, thanks!

Committed and pushed to 8.x. Nice and refreshing to have that done! :)

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