Task #86896
closedApply CGL consistently in .yaml files
Added by Sybille Peters about 6 years ago. Updated about 2 years ago.
100%
Description
The CGL page for Yaml files has been added: https://docs.typo3.org/typo3cms/CoreApiReference/CodingGuidelines/CglYaml.html
The indenting with 2 spaces appears to be applied consistently in the core, but using double quotes ("") or ('') for strings is not.
double quotes:
- rte_ckeditor
single quotes:
- sites: generated config.yaml
- form
- typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml
- ... all others I checked.
Updated by Sybille Peters about 6 years ago
- Category deleted (
System/Bootstrap/Configuration)
Updated by Sybille Peters about 5 years ago
.yaml <> .yml¶
Also, there are a lot of .yaml files:
typo3/sysext/backend/Tests/Functional/Fixtures/CommonScenario.yaml typo3/sysext/core/Tests/Unit/Configuration/Loader/Fixtures/Subfolder/BaseTestInclusion.yaml typo3/sysext/core/Tests/Unit/Configuration/Loader/Fixtures/Berta.yaml typo3/sysext/core/Tests/Unit/Configuration/Fixtures/SiteConfigs/config2.yaml typo3/sysext/core/Tests/Unit/Configuration/Fixtures/SiteConfigs/include.yaml typo3/sysext/core/Tests/Unit/Configuration/Fixtures/SiteConfigs/config1_expected.yaml typo3/sysext/core/Tests/Unit/Configuration/Fixtures/SiteConfigs/config1.yaml typo3/sysext/core/Tests/Unit/Configuration/Fixtures/SiteConfigs/config2_expected.yaml typo3/sysext/core/Tests/Unit/DependencyInjection/Fixtures/Package3/Configuration/Services.yaml typo3/sysext/core/Tests/Unit/DependencyInjection/Fixtures/Package1/Configuration/Services.yaml typo3/sysext/core/Tests/Unit/DependencyInjection/Fixtures/Package4Cycle/Configuration/Services.yaml typo3/sysext/core/Tests/Unit/DependencyInjection/Fixtures/Package2/Configuration/Services.yaml typo3/sysext/core/Tests/Functional/Fixtures/Extensions/irre_tutorial/Configuration/ExtensionBuilder/settings.yaml typo3/sysext/form/Tests/Unit/Mvc/Persistence/Fixtures/BlankForm.form.yaml typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues2.yaml typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/OverruleNonArrayValuesOverArrayValues1.yaml typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/Header.yaml typo3/sysext/form/Tests/Unit/Mvc/Configuration/Fixtures/Invalid.yaml typo3/sysext/form/Tests/Unit/Controller/Fixtures/BlankForm.yaml typo3/sysext/form/Tests/Unit/Controller/Fixtures/SimpleContactForm.yaml typo3/sysext/form/Tests/Functional/Hooks/Fixtures/test_resources/Configuration/Yaml/AllowedExtensionPaths.yaml typo3/sysext/form/Tests/Functional/Hooks/Fixtures/test_resources/Configuration/Form/updated.form.yaml typo3/sysext/form/Tests/Functional/Hooks/Fixtures/test_resources/Configuration/Form/legacy.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/SlugScenario.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/PlainScenario.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioC.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioF.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioB.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioA.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioD.yaml typo3/sysext/frontend/Tests/Functional/SiteHandling/LocalizedPageRendering/Fixtures/ScenarioE.yaml
and a few .yml files:
typo3/sysext/seo/Tests/Functional/Fixtures/HrefLangScenario.yml typo3/sysext/core/Tests/Acceptance/Install.suite.yml typo3/sysext/core/Tests/Acceptance/Backend.suite.yml typo3/sysext/core/Tests/codeception.yml
The CGL currently states .yaml as preferred (though in the field .yml seeems to be used dominantly, see also https://stackoverflow.com/questions/21059124/is-it-yaml-or-yml)
Updated by Sybille Peters over 4 years ago
This might not really be doable, if some third party code expects .yml and we currently use .yaml. I noticed it is just in some of the Tests where .yml exists.
@Christian could you check this? If this is the case, we should probably close the issue.
Updated by Larry Garfield over 2 years ago
I poked around a bit here.
What's unclear to me from the current YAML CG page is if the intent is "if you're going to quote a string, favor single quotes" or "all strings must use single quotes, not bare strings". (YAML allows bare strings in most cases.)
I checked for various other projects, and essentially none have the second policy. There's one that says "quote everything, except for these common cases", which seems like a bad approach for us.
Assuming the intent is the former, we should clarify on the docs page.
By my IDE's count, there are 154 YAML files in core today, and 362 " marks. Some of them are definitely correct (either they are doing interpolation of some kind or they're an HTML string in a multi-line string). An automated solution is unlikely to be applicable here, so that means just a manual review to find excessive " marks.
I have also been unable to find any linting tools that can automate this process to integrate into CI, so we should expect some "style rot" here no matter what we do.
That leaves as my recommended action items:
- Clarify on the docs page that we mean "if you're going to quote, use single quotes".
- Manually grind through every " in the current YAML files and fix them if appropriate; at the current scale that will be annoying but doable in an hour or three.
Unless anyone objects, I will do the above two steps shortly.
Updated by Gerrit Code Review over 2 years ago
- Status changed from New to Under Review
Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74326
Updated by Gerrit Code Review over 2 years ago
Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74326
Updated by Gerrit Code Review over 2 years ago
Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/74326
Updated by Larry Garfield over 2 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 2e5d092e090589b77597ff7568f4691d420f6ccb.